Skip to content

chore: combined PR clean-up - #2164

Merged
profnandaa merged 9 commits into
masterfrom
mcfix/combined-prs
Feb 2, 2023
Merged

chore: combined PR clean-up#2164
profnandaa merged 9 commits into
masterfrom
mcfix/combined-prs

Conversation

@profnandaa

@profnandaa profnandaa commented Jan 31, 2023

Copy link
Copy Markdown
Member

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 <jfml_97@live.com>
@profnandaa
profnandaa marked this pull request as draft January 31, 2023 05:49
@codecov

codecov Bot commented Jan 31, 2023

Copy link
Copy Markdown

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 <tarnai.szabolcs01@gmail.com>
profnandaa and others added 2 commits January 31, 2023 22:20
clean-up #1932 
---------

Co-authored-by: Mik <37216885+mik7up@users.noreply.github.com>
- fixes #2040
- clean-up #2041 

Co-authored-by: coolbeatz71 <sigmacool@gmail.com>
Co-authored-by: Alvaro Castro <moltar89@gmail.com>

@profnandaa: clean up #2103
Comment thread src/lib/isLicensePlate.js
fix(isEmail): fixed `isFQDN` still checking email length when
 `ignore_max_length` is `true`

profnandaa: clean-up #2128

---------

Co-authored-by: Said Akhmedbayev <said.akh@gmail.com>
Co-authored-by: Said Akhmedbayev <said.akhmedbayev@accenture.com>
@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
Comment thread src/lib/isPassportNumber.js Outdated
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
Copy Markdown
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
Copy Markdown
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!

Comment thread src/lib/isMobilePhone.js
'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
Copy Markdown
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
Copy Markdown
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

Comment thread src/lib/isMobilePhone.js
'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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 <digambartu@gmail.com>
* maintenance: clean up #2061

---------

Co-authored-by: djeks922 <djeks922@gmail.com>
Co-authored-by: Frederike Ramin <frederike.ramin@digitalservice.bund.de>
@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