-
Notifications
You must be signed in to change notification settings - Fork 19
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
PE-4565: Address the tech debt created by Turbo Promo Code #1370
Conversation
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.
Self-review checkpoint.
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.
For the reviewer - I just split the method into smaller ones. I've set the nullable parameters to be required so I don't miss passing the argument.
@@ -36,21 +42,6 @@ class PaymentFormLoaded extends PaymentFormState { | |||
final bool isFetchingPromoCode; | |||
final bool errorFetchingPromoCode; | |||
|
|||
double? get promoDiscountFactor { |
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.
For the reviewer - this code was being unused.
Visit the preview URL for this PR (updated for commit c78f9aa): https://ardrive-web--pr1370-pe-4565-ou9cns27.web.app (expires Tue, 17 Oct 2023 15:10:28 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: a224ebaee2f0939e7665e7630e7d3d6cd7d0f8b0 |
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.
LGTM
Follow-up work of #1349 - See the open comments in @thiagocarvalhodev's review.
--- Releases ---
Android release: https://appdistribution.firebase.google.com/testerapps/1:305132849030:android:6cf0cd5ec064fad3ffce07/releases/51pkhnuc5rnto