Conversation
Codecov Report
@@ Coverage Diff @@
## master #1511 +/- ##
=======================================
Coverage 99.92% 99.92%
=======================================
Files 96 96
Lines 1277 1278 +1
=======================================
+ Hits 1276 1277 +1
Misses 1 1
Continue to review full report at Codecov.
|
Related: #15101
profnandaa
left a comment
profnandaa
left a comment
There was a problem hiding this comment.
LGTM, thanks for your contrib! 🎉
|
Please fix the merge conflicts. /cc. @ezkemboi -- kindly review. |
tux-tn
left a comment
tux-tn
left a comment
There was a problem hiding this comment.
Thank you for your contrib! Can you check my comments concerning the decimal separator and the regex
|
|
||
| // Source: https://en.wikipedia.org/wiki/Decimal_mark | ||
| export const dotDecimal = ['ar-EG', 'ar-LB', 'ar-LY']; | ||
| export const dotDecimal = ['ar-EG', 'ar-LB', 'ar-LY', 'fr-CA']; |
There was a problem hiding this comment.
According to many sources (even the link in the code comment) francophone area of Canada (fr-CA) are using decimal comma separator. Can you check this?
There was a problem hiding this comment.
Your're right I made a mistake. I'm in Quebec (canada-french) and most people i work with hate decimals for numbers. I was sure we had the same standard as English.
So yes, in french this is the decimal and not the point. Sorry
| 'es-ES': /^[A-ZÁÉÍÑÓÚÜ]+$/i, | ||
| 'fa-IR': /^[ابپتثجچحخدذرزژسشصضطظعغفقکگلمنوهی]+$/i, | ||
| 'fr-FR': /^[A-ZÀÂÆÇÉÈÊËÏÎÔŒÙÛÜŸ]+$/i, | ||
| 'fr-CA': /^[A-ZÀÂÆÇÉÈÊËÏÎÔŒÙÛÜŸ]+$/i, |
There was a problem hiding this comment.
Since it's the same regex for fr-FR and fr-CA may be we should just assign the value like it was done for pt-BR and all arabic locales
| 'el-GR': /^[0-9Α-ω]+$/i, | ||
| 'es-ES': /^[0-9A-ZÁÉÍÑÓÚÜ]+$/i, | ||
| 'fr-FR': /^[0-9A-ZÀÂÆÇÉÈÊËÏÎÔŒÙÛÜŸ]+$/i, | ||
| 'fr-CA': /^[0-9A-ZÀÂÆÇÉÈÊËÏÎÔŒÙÛÜŸ]+$/i, |
There was a problem hiding this comment.
Same here, doing alphanumeric['fr-CA'] = alphanumeric['fr-FR'] would be easier/better since it's the same regex
|
However, I am on preventive leave due to COVID cases in my family, so I will not be able to correct this in a few days. Can you make the changes? Also, thanks for the review and the suggestions. |
|
Sorry, i don't have write access on master in order to make changes to your Pull request. I guess it can wait few days. Stay safe and take care! |
|
Actually you can create a remote pointing to them and then pull, make
changes and create a PR. Remember to attribute them in the commit so as to
track contribution.
On Fri, Nov 20, 2020 at 6:50 PM Sarhan Aissi ***@***.***> wrote:
Sorry, i don't have write access on master in order to make changes to
your Pull request. I guess it can wait few days. Stay safe and take care!
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1511 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAB7ZENC2KOFYJ5BIJ4PFZTSQ2F5ZANCNFSM4TEFUNVQ>
.
--
Sent from a tiny device while on the move.
|
|
I didn't know that, thank you for the info @profnandaa |
|
@tux-tn -- however, you will need to make a separate PR |
profnandaa
added
the
🧹 needs-update
|
@profnandaa already did |
|
closing in favor of #1528 |
3 participants
Related: #1510
{{ briefly describe what you have done in this PR }}
Checklist