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

Consistency in underscores #30

Merged
merged 6 commits into from
Sep 6, 2018

Conversation

sethherr
Copy link
Contributor

@sethherr sethherr commented Aug 9, 2018

It doesn't make sense to have all the responses use snake case (underscores) with endpoints with dashes.

Partially because consistency but also because most web frameworks use underscores. The only one I'm aware of that uses dashes is Django, and that seems somewhat contentious

@sethherr
Copy link
Contributor Author

I just switched to deregister vehicle, since that change was merged in #31

Personally, I don't think it makes sense to have the header be "deregister" when the endpoint is "remove" though

@hunterowens
Copy link
Collaborator

I'm used to dashes, but I also used to write stuff in Django so maybe that's why. Requesting @toddapetersen to review

@hunterowens
Copy link
Collaborator

@sethherr we're gonna go ahead and use underscores.

We've made the appropriate changes in the provider spec, however, Agency is still behind. Would you mind rebasing and pushing that to us and we can merge in?

@sethherr
Copy link
Contributor Author

sethherr commented Sep 6, 2018

Should be fixed!

Copy link
Collaborator

@hunterowens hunterowens left a comment

Choose a reason for hiding this comment

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

Thanks @sethherr

@hunterowens hunterowens merged commit 7d5a278 into openmobilityfoundation:master Sep 6, 2018
@sethherr sethherr deleted the patch-1 branch September 6, 2018 21:06
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