Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 country-specific test card numbers for credit card testing #9688
Add country-specific test card numbers for credit card testing #9688
Changes from 4 commits
95e6656
64d32ab
4bfaf0e
2062258
5712fbb
11a914b
4ae3dbb
3fc0170
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
What do you think of using the country codes in
Country_Code
for consistency? (you'll probably need to flip theinclude_once
inincludes/class-wc-payments.php
)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.
Makes sense, applied in 11a914b but let me know if I misunderstood your comment and you meant something different.
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.
That's great, thank you! Just this one missing:
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 find it a little bit confusing to type the argument's
$account_country
asstring
(both with PHP 7.0 typing and with the PHP docblock), but settingnull
as its default value.A few questions:
$account_country
benull
or need a default value?get_account_country()
will returnUS
in case ofnull
value:woocommerce-payments/includes/class-wc-payments-account.php
Line 2300 in 2062258
null
be also typed in the docblock?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 agree, no nullable default is needed. I removed it in 5712fbb