Skip to content

fix(isEmail): fixed isFQDN still checking email length when ignore_max_length is true #2128

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

fix(isEmail): fixed isFQDN still checking email length when ignore_max_length is true #2128

wants to merge 2 commits into from

Conversation

@sakhmedbayev
Copy link
Contributor

@sakhmedbayev sakhmedbayev commented Dec 14, 2022

Fixed the issue when the ignore_max_length option was not honored (did not have any effect) since the isFQDN function had its own check for the email parts length.

Checklist

  • PR contains only changes related; no stray files, etc.
  • Tests written (where applicable)

@sakhmedbayev sakhmedbayev changed the title fix(isEmail): fixed isFQDN still checking email length when ignore_max_length is false fix(isEmail): fixed isFQDN still checking email length when ignore_max_length is true Dec 14, 2022
WikiRik
WikiRik requested changes Dec 14, 2022
View reviewed changes
Copy link
Member

@WikiRik WikiRik left a comment

Choose a reason for hiding this comment

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

Code changes look good, don't forget to document the new option for isFQDN in the README

@sakhmedbayev
Copy link
Contributor Author

sakhmedbayev commented Dec 14, 2022

Code changes look good, don't forget to document the new option for isFQDN in the README

@WikiRik, thanks, documented

profnandaa
profnandaa approved these changes Jan 28, 2023
View reviewed changes
Copy link
Member

@profnandaa profnandaa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix! 🎉

@profnandaa profnandaa added mc-to-land Just merge-conflict standing between the PR and landing. 🎉 first-pr PR/combined labels Jan 28, 2023
@profnandaa profnandaa mentioned this pull request Feb 1, 2023
Merged
profnandaa added a commit that referenced this pull request Feb 1, 2023
@profnandaa @saidakhaccenture
fix(isEmail): fixed isFQDN still checking email length when `ignore…
e6e78de 
…_max_length` is `true` (#2170)

profnandaa: clean-up #2128

---------

Co-authored-by: Said Akhmedbayev <[email protected]>
@profnandaa
Copy link
Member

profnandaa commented Feb 1, 2023

combined in #2170

@profnandaa profnandaa closed this Feb 1, 2023
profnandaa added a commit that referenced this pull request Feb 1, 2023
@profnandaa @sakhmedbayev @saidakhaccenture
fix(isEmail): fixed isFQDN's ignore_max_length check (#2170)
b489355 
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@profnandaa profnandaa profnandaa approved these changes

@rubiin rubiin rubiin approved these changes

@WikiRik WikiRik WikiRik approved these changes

Assignees

No one assigned

Labels

🎉 first-pr ✅ LGTM mc-to-land Just merge-conflict standing between the PR and landing. PR/combined

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sakhmedbayev @profnandaa @rubiin @WikiRik @saidakhaccenture