-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat: Add fr-CA locale #1511
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
feat: Add fr-CA locale #1511
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
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, thanks for your contrib! 🎉
|
Please fix the merge conflicts. /cc. @ezkemboi -- kindly review. |
tux-tn
left a comment
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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.
Of course.
| '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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, doing alphanumeric['fr-CA'] = alphanumeric['fr-FR'] would be easier/better since it's the same regex
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.
Of course²
|
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 already did |
|
closing in favor of #1528 |
Related: #1510
{{ briefly describe what you have done in this PR }}
Checklist