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

view for user Registration created #33

Merged
merged 11 commits into from
May 12, 2019
Merged

view for user Registration created #33

merged 11 commits into from
May 12, 2019

Conversation

ayush1999guptA
Copy link
Member

@ayush1999guptA ayush1999guptA commented May 9, 2019

@NeuralFlux
Copy link
Member

May I know what's the difference in between this PR and the previous PR, and why did you close it?

@NeuralFlux
Copy link
Member

Also, the closes #issue should be in the message, not the commit. For eg. The way I edited your comment for this PR.

@NeuralFlux NeuralFlux self-requested a review May 9, 2019 11:32
Copy link
Member

@NeuralFlux NeuralFlux left a comment

Choose a reason for hiding this comment

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

Can you try and validate the email better? Better as in, the current form only checks for the syntax of the email address, whereas, the better one should actually check if the address exists. There are many Python packages to do this, can you try looking into it?

@pep8speaks
Copy link

pep8speaks commented May 9, 2019

Hello @ayush1999guptA! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-05-09 15:21:50 UTC

@ayush1999guptA
Copy link
Member Author

Sorry I forgot to push my changes

@NeuralFlux NeuralFlux merged commit 1764b59 into oss2019:master May 12, 2019
@NeuralFlux
Copy link
Member

Great work on the registration and validation of emails there! But the email registration wasn't how it was supposed to be. So I removed it from this PR message. I'll just update the issue to make it clearer about what's to be done.

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.

Validate Email while User Registers Create a View to Register Users
3 participants