added support for PUT to add body in Curl #657

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
3 participants
@fellars
Contributor

fellars commented Sep 25, 2012

In Curl, if method was set to PUT, it was not adding the body. This checks for a PUT method similar to POST and adds appropriate Curl Options

@gwoo

This comment has been minimized.

Show comment Hide comment
@gwoo

gwoo Sep 26, 2012

Contributor

Could you add a test case for testSendPutAndGet? Have a look at the POST example. https://github.com/UnionOfRAD/lithium/blob/master/tests/cases/net/socket/CurlTest.php#L156

Contributor

gwoo commented Sep 26, 2012

Could you add a test case for testSendPutAndGet? Have a look at the POST example. https://github.com/UnionOfRAD/lithium/blob/master/tests/cases/net/socket/CurlTest.php#L156

@nateabele

This comment has been minimized.

Show comment Hide comment
@nateabele

nateabele Sep 26, 2012

Owner

Yeah, good patch! Once we get a test case in and you squash the commits, we'll merge it for the next release.

Owner

nateabele commented Sep 26, 2012

Yeah, good patch! Once we get a test case in and you squash the commits, we'll merge it for the next release.

@nateabele

This comment has been minimized.

Show comment Hide comment
@nateabele

nateabele Sep 26, 2012

Owner

Oh, hah, I see you added a test case between when I loaded the page and when I left the comment. Can you do me a favor and just squash it down to one commit? Thanks!

Owner

nateabele commented Sep 26, 2012

Oh, hah, I see you added a test case between when I loaded the page and when I left the comment. Can you do me a favor and just squash it down to one commit? Thanks!

@fellars

This comment has been minimized.

Show comment Hide comment
@fellars

fellars Sep 26, 2012

Contributor

I think I squashed into 1. Let me know if I did it wrong.

Contributor

fellars commented Sep 26, 2012

I think I squashed into 1. Let me know if I did it wrong.

@nateabele

This comment has been minimized.

Show comment Hide comment
@nateabele

nateabele Sep 26, 2012

Owner

It's still two (you can tell by hitting the Commits tab of the PR). Here's a simple run-down: http://davidwalsh.name/squash-commits-git

Owner

nateabele commented Sep 26, 2012

It's still two (you can tell by hitting the Commits tab of the PR). Here's a simple run-down: http://davidwalsh.name/squash-commits-git

@fellars

This comment has been minimized.

Show comment Hide comment
@fellars

fellars Sep 26, 2012

Contributor

Alright; I was missing the –f on push

From: Nate Abele [mailto:notifications@github.com]
Sent: Tuesday, September 25, 2012 9:28 PM
To: UnionOfRAD/lithium
Cc: Dan Fellars
Subject: Re: [lithium] added support for PUT to add body in Curl (#657)

It's still two (you can tell by hitting the Commits tab of the PR). Here's a simple run-down: http://davidwalsh.name/squash-commits-git


Reply to this email directly or view it on GitHub #657 (comment) .

https://github.com/notifications/beacon/J6T91GIPIyhU-8ti4GCGPx86vDBZBiWPwftWIkK5t6rdich7P5aGcANcRznLFUAi.gif

Contributor

fellars commented Sep 26, 2012

Alright; I was missing the –f on push

From: Nate Abele [mailto:notifications@github.com]
Sent: Tuesday, September 25, 2012 9:28 PM
To: UnionOfRAD/lithium
Cc: Dan Fellars
Subject: Re: [lithium] added support for PUT to add body in Curl (#657)

It's still two (you can tell by hitting the Commits tab of the PR). Here's a simple run-down: http://davidwalsh.name/squash-commits-git


Reply to this email directly or view it on GitHub #657 (comment) .

https://github.com/notifications/beacon/J6T91GIPIyhU-8ti4GCGPx86vDBZBiWPwftWIkK5t6rdich7P5aGcANcRznLFUAi.gif

@nateabele

This comment has been minimized.

Show comment Hide comment
@nateabele

nateabele Sep 26, 2012

Owner

I think it worked, but I guess the PR doesn't automatically update the commit range, because now there's 4, go figure. Anyway, I just pulled down your fork manually, and cherry picked 95a249d into dev, so we should be all set. Thanks again!

Owner

nateabele commented Sep 26, 2012

I think it worked, but I guess the PR doesn't automatically update the commit range, because now there's 4, go figure. Anyway, I just pulled down your fork manually, and cherry picked 95a249d into dev, so we should be all set. Thanks again!

@nateabele nateabele closed this Sep 26, 2012

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