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

Resourceful routes for users api #5433

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

AntonKhorev
Copy link
Collaborator

@AntonKhorev AntonKhorev commented Dec 21, 2024

Moves user/details and user/gpx_files to their own controllers.

@AntonKhorev AntonKhorev added refactor Refactoring, or things that work but could be done better api Related to the XML or JSON APIs labels Dec 21, 2024
Copy link
Member

@tomhughes tomhughes left a comment

Choose a reason for hiding this comment

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

This mostly looks fine. I think the only thing I'm not sure about is moving user#details into a separate controller - it's basically an alias for user#show so does it make any sense to separate it?

That said @gravitystorm has a better grasp than me of what is the canonical rails to do these things so I'd be interest to here what he thinks.

@AntonKhorev AntonKhorev force-pushed the user-routes-api-namespace branch from cf784a8 to 971fbc2 Compare December 27, 2024 02:51
@github-actions github-actions bot removed the big-pr label Dec 27, 2024
@AntonKhorev
Copy link
Collaborator Author

Let's keep user/details non-resourceful for now, I'll try something else later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Related to the XML or JSON APIs refactor Refactoring, or things that work but could be done better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants