Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reorder controller methods to enhance developer experience and improve readability #90

Conversation

pinzonjulian
Copy link

@pinzonjulian pinzonjulian commented Sep 13, 2022

First of all, thanks for this guide. It's been super interesting going through it

I found the order of controller actions very strange because usually they go in pairs:

  • new with create
  • edit with update

And from a developer perspective, you build a new template (first) to be used by the create action (second). Having it backwards or sometimes mixed up is kind of strange.

The Rails scaffold generator uses another order where it put puts all GET requests first and all others second. However I find this one a bit more aligned with the developer's experience

Copy link
Owner

@stevepolitodesign stevepolitodesign left a comment

Choose a reason for hiding this comment

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

@pinzonjulian thank you for taking the time to make this legibility improvement. The reason the controller actions appear to be in an unconventional order is because they're simply ordered alphabetically. However, I think there's value in placing the similar methods together.

@stevepolitodesign
Copy link
Owner

@pinzonjulian are you able to rebase ontop of main and push up again? I'm unable to trigger CI in my end.

@pinzonjulian
Copy link
Author

Hey Steve, I've closed this because I'm devoting my time to other projects at the moment and this was sort of just an aesthetic thing.

Thanks again for this project!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants