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

Add support for API version 2019-03-14 #811

Merged
merged 1 commit into from
Mar 19, 2019

Conversation

remi-stripe
Copy link
Contributor

@remi-stripe remi-stripe commented Mar 15, 2019

The changes in the library itself look minor. But this API version changes the behavior of Subscriptions entirely under the hood. If the charge fails, you still get a Subscription back instead of an exception and it's on you to check the Subscription status.

This is now fully ready to be reviewed. It also changes the apiversion internal-only constant to a public one.

r? @brandur-stripe
cc @stripe/api-libraries

Related to stripe/stripe-dotnet#1552

@brandur-stripe
Copy link
Contributor

The changes in the library itself look minor. But this API version changes the behavior of Subscriptions entirely under the hood. If the charge fails, you still get a Subscription back instead of an exception and it's on you to check the Subscription status.

Oh, very good to know! Thanks.

LGTM.

@remi-stripe remi-stripe force-pushed the remi-add-new-api-version branch from dadec4e to c4f0007 Compare March 19, 2019 02:51
@remi-stripe remi-stripe force-pushed the remi-add-new-api-version branch from c4f0007 to 8f80019 Compare March 19, 2019 04:02
@remi-stripe
Copy link
Contributor Author

@brandur-stripe I ended up finding a bug in the library which I fixed in #814. After that one is merged, we can properly force the new stripe-mock flag to test the API version in the library is the correct one.

Because of this, apiVersion now has to be exported so I renamed it.

PTAL

@brandur-stripe
Copy link
Contributor

Because of this, apiVersion now has to be exported so I renamed it.

IMO, this is okay. It may be useful to print informational logs or something in client applications anyway. As long as it stays as a const, it can't be abused (by people overwriting it to a new value, etc.).

@brandur-stripe brandur-stripe merged commit 49eb31a into master Mar 19, 2019
@brandur-stripe brandur-stripe deleted the remi-add-new-api-version branch March 19, 2019 16:18
@brandur-stripe
Copy link
Contributor

Released as 58.0.0.

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

Successfully merging this pull request may close these issues.

4 participants