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.

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

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

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

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

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

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

@WikiRik WikiRik WikiRik approved these changes

@rubiin rubiin rubiin approved these changes

@profnandaa profnandaa profnandaa 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