-
Notifications
You must be signed in to change notification settings - Fork 309
Update README to list all new payment methods #1664
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
Conversation
Dependency Review✅ No vulnerabilities found.Scanned FilesNone |
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.
Looks like this is missing the data collector as well
README.md
Outdated
- [ThreeDSecure](https://developer.paypal.com/braintree/docs/guides/3d-secure/overview) | ||
- [Local Payment Methods](https://developer.paypal.com/braintree/articles/guides/payment-methods/local-payment-methods) | ||
- [SEPA Direct Debit](https://developer.paypal.com/braintree/docs/guides/sepa-direct-debit/overview/) | ||
- [AMEX](https://developer.paypal.com/braintree/articles/au/transactions/accepted-payment-methods#american-express) |
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.
- [AMEX](https://developer.paypal.com/braintree/articles/au/transactions/accepted-payment-methods#american-express) | |
- [American Express](https://developer.paypal.com/braintree/articles/au/transactions/accepted-payment-methods#american-express) |
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.
Curious if this change is necessary or a preference? I think AMEX is fine
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.
We do not call it AMEX in the SDK or anywhere in the dev docs, so it's important to align the naming here with what we publicly advertise as the product name.
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.
I'll make the change but we do use both Amex and American Express in both the SDK and the dev docs
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.
Since we call the module by the full name, we should use that here for consistency across the product: https://github.com/braintree/braintree_ios/tree/main/Sources/BraintreeAmericanExpress
Curious why data collector should be listed as a payment method? It would be it's own separate section. I know it's one of the BT integrations but the definition and purpose of it is "DataCollector is used to collect device information to aid in fraud detection and prevention." which is why I didn't include it. |
That's a good call! Since we want this to represent all features and have added Shopper Insights in this PR we should rename the header from |
Updated README to include all the payment methods we offer.
Summary of changes
Checklist
Authors