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

Use uppercase methods in README #272

Merged
merged 1 commit into from
Mar 31, 2016

Conversation

jamesplease
Copy link
Contributor

HTTP methods are case-sensitive. The examples in the README currently
lead developers to think that this API normalizes all of the method
names to uppercase, leading to issues like #37 and #254.

Setting the right precedent in the README will prevent developers
from making mistake of trying to use method: 'patch' (or any
other verb that isn't normalized for backwards compatibility).

@jamesplease
Copy link
Contributor Author

I tried to find the GitHub CLA on the interwebs, but couldn't find it. If someone sends me a link I can get that signed.

HTTP methods are case-sensitive. The examples in the README currently
lead developers to think that this API normalizes all of the method
names to uppercase, leading to issues like JakeChampion#37 and JakeChampion#254.

Setting the right precedent in the README will prevent developers
from making mistake of trying to use method: 'patch' (or any
other verb that isn't normalized for backwards compatibility).
@jamesplease jamesplease changed the title Capitalize methods in README Use uppercase methods in README Jan 31, 2016
@emilgpa
Copy link

emilgpa commented Jan 31, 2016

I think that is here: https://cla.github.com

@jamesplease
Copy link
Contributor Author

Thanks @DasHaus ! Now I'm really embarrassed. I can't figure out how to sign the thing on the page. I thought maybe there was a JavaScript timer on the page hiding the accept button for a minute, but it's not sending over any JS.

I'm wondering if this empty div is where the sign button is supposed to show up?

image

@jamesplease
Copy link
Contributor Author

Oh, figured it out! I needed to click through to the CLA from the failed PR status. Kinda weird, but hey, I figured it out ✌️

@dgraham
Copy link
Contributor

dgraham commented Jan 31, 2016

The case of PATCH is indeed unfortunate. Perhaps one day it will be afforded the privileges enjoyed by the original Gang of Four verbs and may be spoken in an inside voice. For now, we must shout it into the computer and hope it reaches the server.

I continue to hope for a patch future.

@dgraham dgraham closed this Jan 31, 2016
@jamesplease
Copy link
Contributor Author

@dgraham I, too, share a vision with you for patch! But alas, it may be some time before we get there. In the meantime, I think we have a responsibility to do what we can to prevent developers from spending time figuring out why (seemingly arbitrary) HTTP verbs don't work with this library.

I think it's a problem that the README sets developers up to fail right now. If they're like me, they might copy + paste an example, then modify it for their own request. Unfortunately, nothing in the README explains the important detail that only some of the verbs are normalized.

It's clear from those two issues (#37 and #254) – and my own experience – that some updated documentation could do some good here. Would you be open to a PR that adds a note about case sensitivity to the README?

@jamesplease jamesplease deleted the capitalization branch January 31, 2016 20:42
@jamesplease jamesplease restored the capitalization branch March 25, 2016 18:27
@dgraham dgraham reopened this Mar 31, 2016
@dgraham dgraham merged commit c23c3f9 into JakeChampion:master Mar 31, 2016
@atticoos
Copy link

Thanks @dgraham 👍

@jamesplease
Copy link
Contributor Author

@dgraham , thanks, yo ✌️

@jamesplease jamesplease deleted the capitalization branch March 31, 2016 17:27
@LavaToaster
Copy link

🎉

Thanks @dgraham ❤️

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 2, 2020
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.

5 participants