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

[Link] StripePaymentSheet depends on StripeLinkCore #2351

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

bmelts-stripe
Copy link
Contributor

Summary

Added StripeLinkCore as a dependency of StripePaymentSheet, and updated documentation. cc @kgaidis-stripe

Motivation

Part of the ongoing module creation process (see: #2320, #2335).

Testing

Ran through wiki instructions, everything works as expected.

Notes

I copied the wording in the documentation updates from the wiki, but I'm open to discussion on if there are better ways we should communicate this. I've also left the version undefined for MIGRATING.md - we should decide on what kind of bump we want to do for this (major or minor).

@kgaidis-stripe
Copy link
Contributor

kgaidis-stripe commented Mar 3, 2023

@bmelts-stripe

UPDATE/EDIT: Talked to @vardges-stripe and we do not need to make FinancialConnections depend on StripeLinkCore. We'll do that later.


Does the merging of this PR mean that people will have to perform migration steps IF they use StripePaymentSheet?

Since this is a little bit of a migration commitment, I wonder what you think about trying to do this once for any framework that will eventually depend on StripeLinkCore? Is it just StripePaymentSheet and then eventually FinancialConnections, or are there others? For example, I wonder if Financial Connections should do this now (even if we don't use it at the moment) because we will have to anyway. CC: @vardges-stripe

@jaynewstrom-stripe jaynewstrom-stripe removed their request for review March 9, 2023 20:36
@@ -1,3 +1,7 @@
## 24.0.0 yyyy-mm-dd
Copy link
Contributor

Choose a reason for hiding this comment

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

leave this as x.x.x yyyy-mm-dd the release script will replace it with the right version/date

Copy link
Contributor

@davidme-stripe davidme-stripe left a comment

Choose a reason for hiding this comment

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

Chatted with bmelts offline, we're holding off on a major version bump for now.

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

Successfully merging this pull request may close these issues.

None yet

4 participants