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

cURL: Support custom HTTP methods #227

Merged
merged 3 commits into from
Aug 4, 2016

Conversation

ocean90
Copy link
Member

@ocean90 ocean90 commented Jul 28, 2016

See https://core.trac.wordpress.org/ticket/37503 for background.

The test doesn't work (yet) because there is no /purge endpoint.

@codecov-io
Copy link

codecov-io commented Jul 28, 2016

Current coverage is 92.22% (diff: 100%)

Merging #227 into master will increase coverage by <.01%

@@             master       #227   diff @@
==========================================
  Files            21         21          
  Lines          1761       1762     +1   
  Methods         156        156          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1624       1625     +1   
  Misses          137        137          
  Partials          0          0          

Powered by Codecov. Last update 17c15b0...aec05fe

@rmccue
Copy link
Collaborator

rmccue commented Jul 28, 2016

https://github.com/RequestsPHP/test-server is the source for the test server. :)

ocean90 added a commit to ocean90/test-server that referenced this pull request Jul 28, 2016
@ocean90
Copy link
Member Author

ocean90 commented Jul 30, 2016

@rmccue No idea why it returns a 501 error. :(

@ocean90
Copy link
Member Author

ocean90 commented Aug 3, 2016

Instead of adding a new case we can also combine the default case with PATCH, PUT, DELETE and OPTIONS. See https://core.trac.wordpress.org/attachment/ticket/37503/37503.2.patch.

@rmccue Let me know if this is a better approach, happy to refresh the PR. (It would also fix the coverage thing. :))

@rmccue
Copy link
Collaborator

rmccue commented Aug 3, 2016

Coverage can be pretty safely ignored for <0.1%; I tried this PR locally and it worked, but not on Travis, so need to get to the bottom of that. Haven't had time yet, alas.

I'd prefer the switch be explicit in what it supports, but case ...: default: is fine to avoid the duplication and still be explicit.

@rmccue
Copy link
Collaborator

rmccue commented Aug 4, 2016

Tests passing, wunderbar!

@rmccue rmccue added this to the 1.7 milestone Aug 4, 2016
@rmccue rmccue merged commit 1b5ffd8 into WordPress:master Aug 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants