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

Updating a card is a POST action, not PUT #63

Closed
nathany opened this issue Aug 2, 2016 · 5 comments
Closed

Updating a card is a POST action, not PUT #63

nathany opened this issue Aug 2, 2016 · 5 comments

Comments

@nathany
Copy link

nathany commented Aug 2, 2016

See docs: https://stripe.com/docs/api#update_card

Code at: https://github.com/robconery/stripity-stripe/blob/master/lib/stripe/cards.ex#L81

I was planning to send you a patch, but #62, and the number of open pull requests suggests I should look elsewhere. 😦

@robconery
Copy link
Contributor

I'm busy, just like you. I have a family and summer vacations to juggle and, yeah sometimes these PRs might sit for a few weeks until I can get to them. I'll be honest with you: I don't appreciate the comment.

As far as POST vs. PUT - I'd love to have your help if you had the time because as you can tell, I'm having a hard time pulling in the PRs quickly. Most of this is due to switching over to coveralls and trying to figure out how the builds can run safely.

@nathany
Copy link
Author

nathany commented Aug 3, 2016

Apologies. It was frustrating to be met with a failing build when I have such a small change to make. I understand that it's open source and I shouldn't expect you to drop everything to fix up stripity_stripe.

Thanks for cleaning up the PRs.

I'm not sure I understand the current issue with the build and Coveralls. Is it an issue with specifying the Stripe credentials in such a way that they don't leak?

@gmile
Copy link
Contributor

gmile commented Aug 4, 2016

@nathany the build is now fixed in #64. Coveralls was not an issue. It usually is an issue only when a PR reduces the % of code coverage – it makes the coveralls , which makes the coveralls red. But that was not the case here.

There actual problem with the build was the fact, that recently we began escaping properly (in #59), while VCR cassettes were recorded back when escaping was done incorrectly. This was addressed in #64 (check out the diff to see what I mean).

@gmile
Copy link
Contributor

gmile commented Aug 4, 2016

@nathany I've fixed this issue in #66. That PR still fails (as well as others I just submitted) until #64 is merged.

@robconery
Copy link
Contributor

Closing for now - cheers all thank you for your help :)>

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

No branches or pull requests

3 participants