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

Remove RequestOptions.setStripeVersionOverride #1464

Merged

Conversation

richardm-stripe
Copy link
Contributor

@richardm-stripe richardm-stripe commented Oct 25, 2022

Notify

cc @stripe/api-library-reviewers

Summary

This PR removes RequestOptions.setStripeVersionOverride. There was only one supported use case for setStripeVersionOverride, for EphemeralKey.create. Now, users should

EphemeralKeyCreateParams params = EphemeralKeyCreateParams.builder()
  .setStripeVersion("XXXX-YY-ZZ")
  .build();

to set the Stripe-Version header appropriately.

We introduce RequestOptions.unsafeSetStripeVersionOverride which is marked as internal only.
Because com.stripe.model.EphemeralKey is not in the com.stripe.net package, we can't actually change make this internal-only method private, but users should not use it.

Changelog

  • Add support for StripeVersion as a required value on EphemeralKeyCreateParams.
  • Add support for EphemeralKey.create without specifying RequestOptions.
  • RequestOptions.setStripeVersionOverride is renamed to unsafeStripeVersionOverride to confront upgrading users with the fact that this is now internal-only and should not be used. The docs now indicate it is internal-only. It is also now a static method, to prevent it from polluting IDE autocomplete list.
  • RequestOptions.getStripeVersionOverride is similarly changed to unsafeGetStripeVersionOverride.
  • RequestOptions.clearStripeVersionOverride is removed entirely. There is no reason to use this.
  • RequestOptions.toBuilder is now marked as deprecated, in favor of toBuilderFullCopy, which has clearer semantics.

@richardm-stripe richardm-stripe changed the title [WIP] Ephemeral key accepts Stripe-Version through params, not opts Ephemeral key accepts Stripe-Version through params, not opts Oct 25, 2022
@richardm-stripe richardm-stripe changed the title Ephemeral key accepts Stripe-Version through params, not opts Ephemeral key accepts Stripe-Version through params, not opts (next major) Oct 25, 2022
@richardm-stripe
Copy link
Contributor Author

Assigned you @pakrym-stripe as you are the "reporter" on our internal ticket for this ;)

* @return option builder
*/
public RequestOptionsBuilder setStripeVersionOverride(String stripeVersionOverride) {
public RequestOptionsBuilder _setStripeVersionOverride(String stripeVersionOverride) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make this a static method so it doesn't show up in IDE code completion. And maybe name it unsafeSetStripeVersionOverride?

Copy link
Contributor

@pakrym-stripe pakrym-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall. Wonder if we can hide _setStripeVersionOverride in a way that doesn't pollute code completion.

Copy link
Contributor

@pakrym-stripe pakrym-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit, let's hide the getVersionOverride

richardm-stripe and others added 2 commits November 4, 2022 13:00
Co-authored-by: pakrym-stripe <99349468+pakrym-stripe@users.noreply.github.com>
@richardm-stripe richardm-stripe merged commit d6ba5f6 into sdk-release/next-major Nov 4, 2022
@richardm-stripe richardm-stripe deleted the richardm-curb-version-overrides branch November 4, 2022 22:55
@richardm-stripe richardm-stripe changed the title Ephemeral key accepts Stripe-Version through params, not opts (next major) Remove RequestOptions.setStripeVersionOverride Nov 7, 2022
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.

None yet

2 participants