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 (webapi): Add enforcing of restrictions for username and email #1587

Merged
merged 4 commits into from
Feb 9, 2020
Merged

fix (webapi): Add enforcing of restrictions for username and email #1587

merged 4 commits into from
Feb 9, 2020

Conversation

subotic
Copy link
Collaborator

@subotic subotic commented Jan 28, 2020 •
edited
Loading

resolves #1585

@subotic subotic added the bug something isn't working label Jan 28, 2020
@subotic subotic added this to the 2020-02 milestone Jan 28, 2020
@subotic subotic self-assigned this Jan 28, 2020
@subotic subotic requested a review from benjamingeer January 28, 2020 12:54
@subotic
Copy link
Collaborator Author

subotic commented Jan 31, 2020

@benjamingeer whenever you have a spare moment, a review would be great. Thanks!

benjamingeer
benjamingeer approved these changes Jan 31, 2020
View reviewed changes
Copy link

@benjamingeer benjamingeer left a comment

Choose a reason for hiding this comment

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

The lookahead and lookbehind syntax in the regex gave me a headache, could you possibly break it up and add comments to explain what each part of the regex is doing? Especially because the Javadoc documentation of java.util.regex.Pattern doesn't explain this syntax. Otherwise looks good, thanks!

@subotic
Copy link
Collaborator Author

subotic commented Jan 31, 2020

Didn't you add the regex? It was there already. I have no idea what it exactly does.

@benjamingeer
Copy link

You added it in this commit as part of PR #1208:

3c4d965#diff-659c65d367a0897435d7612e0d1b8a4bR788

I agree that it was there already and so it's not a problem with this PR, but it would be nice to have it explained a bit. If you've already forgotten how it works, that's a good example of why it could use a comment. :)

@subotic subotic modified the milestones: 2020-02, 2020-01 Feb 7, 2020
@subotic
Copy link
Collaborator Author

subotic commented Feb 9, 2020

@benjamingeer Thanks for the review.

@subotic subotic merged commit ccb64bb into develop Feb 9, 2020
@subotic subotic deleted the wip/1585-check-username branch February 9, 2020 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjamingeer benjamingeer benjamingeer approved these changes

Assignees

@subotic subotic

Labels
bug something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Check username for allowed characters when new user is created
2 participants
@subotic @benjamingeer