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

Already on GitHub? Sign in to your account

Added HTTP Patch to HTTParty #131

Merged
merged 24 commits into from Apr 16, 2012

Conversation

Projects
None yet
2 participants

I followed the conventions used for similar requests. I did not do any additional/creative testing, but followed the convention for testing other request types.

Owner

jnunemaker commented Apr 15, 2012

So the problem with this is backwards compatibility. This is 1.9.3 only, isn't it?

A little googling turned up: https://github.com/archiloque/rest-client/issues/79 and https://gist.github.com/1023207

It appears that this method exists as the PROPPATCH method in 1.8.7 and 1.9.1. While I am not sure, I believe that the ruby core team may have used the PROP- prefix to denote proposed HTTP methods. I think @tenderlove may be able to answer such a question.

In 1.9.3 there is both a PROPPATCH and a PATCH, I believe to support backwards compatibility.

Nevermind. It appears to in support of a different method.

I sent patches to 1.8.7 and 1.9.2. Hopefully we can move past the backwards compatibility stuff if they get accepted.

Would it be better to monkey patch it until the patches go through?

Owner

jnunemaker commented Apr 15, 2012

Yep. Either way we'll have to monkey patch. Not everyone would be running on latest if those patches were accepted.

On Apr 15, 2012, at 12:48 PM, Isaac Sandersreply@reply.github.com wrote:

Would it be better to monkey patch it until the patches go through?


Reply to this email directly or view it on GitHub:
#131 (comment)

Ok, I will get my capuchin going.

This is the implementation that I submitted to ruby-core. It works with 1.8.7 and 1.9.2. It also makes PATCH specs that I have for my githubris project pass.

Owner

jnunemaker commented Apr 15, 2012

Hmm, looks like there is a failure on master. I'll get that fix and then pull this.

The travis config happened here.... I will make a new branch. #133

@jnunemaker jnunemaker merged commit d51884c into jnunemaker:master Apr 16, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment