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 vertical layout toggle to playground #3556

Merged
merged 3 commits into from
May 3, 2024

Conversation

yuki-stripe
Copy link
Collaborator

Summary

Adds the toggle and a dummy placeholder API.

Future PRs will update the placeholder API to a real (but internal) API and, of course, make the toggle do something :)

Motivation

Keep the PRs small

Copy link

github-actions bot commented May 2, 2024

⚠️ Public API changes detected:

StripePayments

- @objc public var allowRedisplay: StripePayments.STPPaymentMethodAllowRedisplay
- @objc convenience public init(card: StripePayments.STPPaymentMethodCardParams, billingDetails: StripePayments.STPPaymentMethodBillingDetails?, allowRedisplay: StripePayments.STPPaymentMethodAllowRedisplay = .unspecified, metadata: [Swift.String : Swift.String]?)
+ @objc convenience public init(card: StripePayments.STPPaymentMethodCardParams, billingDetails: StripePayments.STPPaymentMethodBillingDetails?, metadata: [Swift.String : Swift.String]?)
- @objc convenience public init(usBankAccount: StripePayments.STPPaymentMethodUSBankAccountParams, billingDetails: StripePayments.STPPaymentMethodBillingDetails, allowRedisplay: StripePayments.STPPaymentMethodAllowRedisplay = .unspecified, metadata: [Swift.String : Swift.String]?)
+ @objc convenience public init(usBankAccount: StripePayments.STPPaymentMethodUSBankAccountParams, billingDetails: StripePayments.STPPaymentMethodBillingDetails, metadata: [Swift.String : Swift.String]?)
- case unspecified
- case limited
- case always
- public init?(rawValue: Swift.Int)
- public typealias RawValue = Swift.Int
- public var rawValue: Swift.Int {
- get
- }

If you are adding a new public API consider the following:

  • Do these APIs need to be public or can they be protected with @_spi(STP)?
  • If these APIs need to be public, assess whether they require an API review.

If you are modifying or removing a public API:

  • Does this require a breaking version change?
  • Do these changes require API review?

If you confirm these APIs need to be added/updated and have undergone necessary review, add the label modifies public API to this PR to acknowledge and bypass this check.

@yuki-stripe yuki-stripe force-pushed the yuki/add-dummy-vertical-mode-to-playground branch from bc52364 to b224235 Compare May 2, 2024 22:31
porter-stripe
porter-stripe previously approved these changes May 2, 2024
Copy link
Collaborator

@porter-stripe porter-stripe left a comment

Choose a reason for hiding this comment

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

I'm guessing the bot is leaving a comment about the public APIs since the branch it out of date w/ master?

@yuki-stripe
Copy link
Collaborator Author

I'm guessing the bot is leaving a comment about the public APIs since the branch it out of date w/ master?

@porter-stripe Hmm I guess so. Should we tweak the script? O/w I'd think this would trigger often.

@porter-stripe
Copy link
Collaborator

porter-stripe commented May 3, 2024

@yuki-stripe I'll look into modifying the script to look at the first commit from when the branch was started rather than current master. But it gets me thinking, maybe we shouldn't allow merging into master if your branch isn't up to date with master. I could imagine a scenario where something could break, and technically the bitrise build is building an out of date build since it doesn't have the latest changes in master, unless it's merging the changes during the build?

@yuki-stripe yuki-stripe merged commit bd2f08c into master May 3, 2024
4 checks passed
@yuki-stripe yuki-stripe deleted the yuki/add-dummy-vertical-mode-to-playground branch May 3, 2024 16:53
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

2 participants