Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

Use model#isPlural to pick which template to auto-render #1728

Merged
merged 2 commits into from
Jul 10, 2019

Conversation

bytheway
Copy link
Contributor

I ran into an issue attempting to use hyphens instead of underscores in resource URLS (Google recommends underscores not be used: https://support.google.com/webmasters/answer/76329?hl=en).

I traced the limitation down to the autorender, which makes assumptions about the url path segment matching the underscore form. Instead, it is possible to know whether to render the index.html or show.html based on whether we have a collection or a single instance of the model.

This allows using dashes instead of underscores in resource urls.
@bytheway bytheway requested a review from a team as a code owner July 10, 2019 13:40
@markbates
Copy link
Member

Can you please add some tests? Thanks.

Also, Go convention is underscores for file names, which is why Buffalo uses underscores.

@bytheway
Copy link
Contributor Author

Happy to add tests, will do that this evening.

I understand the underscore convention for identifiers and file names (this PR doesn't change that), but that convention gets messy when it runs into the more visible url conventions which have their own style guides, especially when it comes to search engines, etc.

Copy link
Member

@markbates markbates left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks. When the tests pass I’ll merge. We’re doing a v0.14.7 release next week before GopherCon, so this will be released then.

@markbates markbates added this to the Next milestone Jul 10, 2019
@markbates markbates merged commit b1f586d into gobuffalo:development Jul 10, 2019
@markbates markbates modified the milestones: Next, v0.14.7 Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants