Skip to content
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

chore: combined PR clean-up #2164

Merged
merged 9 commits into from
Feb 2, 2023
Merged

chore: combined PR clean-up #2164

merged 9 commits into from
Feb 2, 2023

Conversation

profnandaa
Copy link
Member

@profnandaa profnandaa commented Jan 31, 2023

This is a clean-up PR for other PRs that had merge-conflicts, especially on README.

chore: fix merge conflicts for #1555

Co-authored-by: Juan Medina <[email protected]>
@profnandaa profnandaa marked this pull request as draft January 31, 2023 05:49
@codecov
Copy link

codecov bot commented Jan 31, 2023

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (c6f2196) compared to base (6dba289).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #2164   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          105       105           
  Lines         2334      2323   -11     
  Branches       586       586           
=========================================
- Hits          2334      2323   -11     
Impacted Files Coverage Δ
src/lib/isMobilePhone.js 100.00% <ø> (ø)
src/lib/isPassportNumber.js 100.00% <ø> (ø)
src/lib/isEmail.js 100.00% <100.00%> (ø)
src/lib/isFQDN.js 100.00% <100.00%> (ø)
src/lib/isFloat.js 100.00% <100.00%> (ø)
src/lib/isLicensePlate.js 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

chore: clean up PR #1895 
---------

Co-authored-by: szabolcstarnai <[email protected]>
fix(isEmail): fixed `isFQDN` still checking email length when
 `ignore_max_length` is `true`

profnandaa: clean-up #2128

---------

Co-authored-by: Said Akhmedbayev <[email protected]>
Co-authored-by: Said Akhmedbayev <[email protected]>
@profnandaa profnandaa marked this pull request as ready for review February 1, 2023 18:05
@profnandaa profnandaa requested a review from pano9000 February 1, 2023 18:06
@@ -48,12 +51,13 @@ const passportRegexByCountryCode = {
MY: /^[AHK]\d{8}$/, // MALAYSIA
MX: /^\d{10,11}$/, // MEXICO
NL: /^[A-Z]{2}[A-Z0-9]{6}\d$/, // NETHERLANDS
NZ: /^([Ll]([Aa]|[Dd]|[Ff]|[Hh])|[Ee]([Aa]|[Pp])|[Nn])\d{6}$/, // NEW ZELAND
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
NZ: /^([Ll]([Aa]|[Dd]|[Ff]|[Hh])|[Ee]([Aa]|[Pp])|[Nn])\d{6}$/, // NEW ZELAND
NZ: /^([Ll]([Aa]|[Dd]|[Ff]|[Hh])|[Ee]([Aa]|[Pp])|[Nn])\d{6}$/, // NEW ZEALAND

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, fixed, thanks!

@@ -95,6 +95,7 @@ const phones = {
'fo-FO': /^(\+?298)?\s?\d{2}\s?\d{2}\s?\d{2}$/,
'fr-BF': /^(\+226|0)[67]\d{7}$/,
'fr-BJ': /^(\+229)\d{8}$/,
'fr-CD': /^(\+?243|0)?(8|9)\d{8}$/,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'fr-CD': /^(\+?243|0)?(8|9)\d{8}$/,
'fr-CD': /^(\+?243|0)?(8[0-2489]|9[017-9])\d{7}$/,

According to https://www.itu.int/dms_pub/itu-t/oth/02/02/T02020000370001PDFE.pdf with the addition of Africell phone numbers from https://github.com/google/libphonenumber/blob/master/resources/metadata/243/ranges.csv in line with the comment of the creator of the original PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can fix this in a later PR as well

@@ -131,6 +132,7 @@ const phones = {
'pt-BR': /^((\+?55\ ?[1-9]{2}\ ?)|(\+?55\ ?\([1-9]{2}\)\ ?)|(0[1-9]{2}\ ?)|(\([1-9]{2}\)\ ?)|([1-9]{2}\ ?))((\d{4}\-?\d{4})|(9[1-9]{1}\d{3}\-?\d{4}))$/,
'pt-PT': /^(\+?351)?9[1236]\d{7}$/,
'pt-AO': /^(\+244)\d{9}$/,
'ro-MD': /^(\+?373|0)((6(0|1|2|6|7|8|9))|(7(6|7|8|9)))\d{6}$/,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'ro-MD': /^(\+?373|0)((6(0|1|2|6|7|8|9))|(7(6|7|8|9)))\d{6}$/,
'ro-MD': /^(\+?373|0)((6([089]\d{6}|(1[01]|2[01]|7[1-7])\d{5}))|(7((6[07]|8[0-8])\d{5}|9\d{6})))$/

According to https://www.itu.int/dms_pub/itu-t/oth/02/02/T020200008C0003PDFE.pdf
The result is correct but maybe we RegExp might be simplified a bit so I'm open for suggestions on that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can fix this in a later PR as well

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally agree... we can do a subsequent fix for that.

* maintentance: clean up, closes #2073
---------

Co-authored-by: Digambar <[email protected]>
* maintenance: clean up #2061

---------

Co-authored-by: djeks922 <[email protected]>
@profnandaa profnandaa merged commit 91c8bd9 into master Feb 2, 2023
@profnandaa profnandaa deleted the mcfix/combined-prs branch February 2, 2023 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants