-
Notifications
You must be signed in to change notification settings - Fork 3k
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 regex for US account number #7101
add regex for US account number #7101
Conversation
Hey @thesahindia, could you please add QA steps something like - Verify that account number works for numbers of length 4-17 |
@rushatgabhane, updated the QA steps. |
Logs show there's some error with bot's GPG key. |
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 good. Suggested some changes.
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! 🎉
All yours @puneetlath
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.
One small comment. Otherwise looks good.
Looks good @thesahindia. Can you please merge main to fix the failing tests? Context here: https://expensify.slack.com/archives/C01GTK53T8Q/p1641911028289600 |
Merge branch 'main' into thesahindia/ux/add-regex
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by @puneetlath in version: 1.1.27-2 🚀
|
🚀 Deployed to staging by @puneetlath in version: 1.1.27-3 🚀
|
🚀 Deployed to staging by @puneetlath in version: 1.1.27-3 🚀
|
Details
Added regex for US account number
Fixed Issues
$ #7056
Tests
QA Steps
Tested On
Screenshots
Web
Screen.Recording.2022-01-10.at.6.50.13.PM.mov
Mobile Web
Desktop
iOS
Android