-
Notifications
You must be signed in to change notification settings - Fork 31
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
Pay with bat skus #1789
Pay with bat skus #1789
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.
LGTM overall. Just some comments from the API changes to the new managed payments API.
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.
Added a couple of comments on the sku files, plus expressed my feedback in #1892.
tools/macaroon/cmd/brave-firewall-vpn/premium_dev_time_limited_v2_bat.yaml
Show resolved
Hide resolved
tools/macaroon/cmd/brave-firewall-vpn/premium_dev_time_limited_v2_bat.yaml
Show resolved
Hide resolved
tools/macaroon/cmd/brave-firewall-vpn/premium_dev_time_limited_v2_bat.yaml
Outdated
Show resolved
Hide resolved
tools/macaroon/cmd/brave-firewall-vpn/premium_prod_time_limited_v2_bat.yaml
Outdated
Show resolved
Hide resolved
tools/macaroon/cmd/brave-firewall-vpn/premium_stg_time_limited_v2_bat.yaml
Outdated
Show resolved
Hide resolved
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.
Looking good. 👍
…pay-with-bat-skus
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.
Please do not merge. Somehow, the recent conflict resolution brought in back code that has been removed. We need to fix that.
cc @husobee
05ea00d
to
5fd9b51
Compare
fixing test
7d5c506
to
00e49a9
Compare
Thanks @husobee ! I have deleted the code that had to be deleted (some of the fixes have brought in the old implementation of payment methods). Also, addressed some of my comments on gateway management (moved that to the client). As soon as I see tests pass, will re-approve the PR. |
Summary
Type of change ( select one )
Tested Environments
Before submitting this PR:
Manual Test Plan: