-
Notifications
You must be signed in to change notification settings - Fork 25
Migrate CheckoutErrorRequest to CheckoutErrorEvent #493
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
base: prototype/development
Are you sure you want to change the base?
Migrate CheckoutErrorRequest to CheckoutErrorEvent #493
Conversation
d962027 to
a513e09
Compare
| /// Issued when the storefront configuration has caused an error. | ||
| /// Note that the Checkout Sheet Kit only supports stores migrated for extensibility. | ||
| case configurationError(message: String, code: CheckoutErrorCode, recoverable: Bool = false) | ||
| case misconfiguration(message: String, code: ErrorCode, recoverable: Bool = false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proposal: rename configuration -> misconfiguration
Felt closer aligned to the naming of "expired" and "unavailable" (the target of the noun is whats happened?)
.sdk still feels like an outlier in that sense though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.sdk → .internal maybe?
misconfiguration looks good
| /// | ||
| /// Every event has a "recoverable" property that indicates this error may be recoverable when retried in a fallback browser experience | ||
| /// This may have a degraded experience, implement CheckoutDelegate.shouldRecoverFromError to opt out | ||
| public enum CheckoutError: Error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this error class is CheckoutError I thought it may make sense to remove the repeated/inconsistent "checkout" and "error" in the cases
| /// unavailable: recoverable:true | ||
| case killswitchEnabled = "KILLSWITCH_ENABLED" | ||
| case unrecoverableFailure = "UNRECOVERABLE_FAILURE" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
recoverable: true here seems wrong?
What changes are you making?
Before you merge
Important
Releasing a new version of the kit?
podspecfile.Releasing a new major version?
Tip
See the Contributing documentation for instructions on how to publish a new version of the library.