Skip to content
This repository has been archived by the owner on Apr 12, 2022. It is now read-only.

Put email regex back to old, fixed tests #3004

Merged
merged 2 commits into from Feb 25, 2022

Conversation

krishnaglick
Copy link
Contributor

@krishnaglick krishnaglick commented Feb 24, 2022

Change description

Put email regex back to old, fixed tests

Checklists

Development

  • Application changes have been tested appropriately

Impact

  • Code follows company security practices and guidelines
  • Security impact of change has been considered
  • Performance impact of change has been considered
  • Possible migration needs considered (model migrations, config migrations, etc.)

Please explain any security, performance, migration, or other impacts if relevant:

Code review

  • Pull request has a descriptive title and context useful to a reviewer. Screenshots or screencasts are attached where applicable.
  • Relevant tags have been added to the PR (bug, enhancement, internal, etc.)

@krishnaglick krishnaglick added the internal chores and housekeeping label Feb 24, 2022
Comment on lines 5 to 7
* We cannot use import isEmail from "validator/lib/isEmail" until https://github.com/validatorjs/validator.js/pull/1435 is merged in.
*/
export default function isEmail(string: string) {
Copy link
Member

Choose a reason for hiding this comment

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

This comment is waiting on something to be merged that has been merged at this point. We have validator installed... could we just use that?

If not, maybe this comment should be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That'd be an @bleonard call.

Copy link
Member

Choose a reason for hiding this comment

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

I'd remove that comment - we still don't really want to use the more strict email validator from validator.js - they won't like the leading/trailing spaces

Comment on lines 5 to 7
* We cannot use import isEmail from "validator/lib/isEmail" until https://github.com/validatorjs/validator.js/pull/1435 is merged in.
*/
export default function isEmail(string: string) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd remove that comment - we still don't really want to use the more strict email validator from validator.js - they won't like the leading/trailing spaces

@krishnaglick krishnaglick enabled auto-merge (squash) February 25, 2022 15:06
@krishnaglick krishnaglick merged commit fe90d9d into main Feb 25, 2022
@krishnaglick krishnaglick deleted the 181225513-revert-email-validator branch February 25, 2022 15:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
internal chores and housekeeping
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants