specifying reviewer with dot sign in the nickname doesn't work

RESOLVED FIXED

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: simon, Assigned: glob)

Tracking

Details

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20160105164030

Steps to reproduce:

-Make a commit
-type as reviewer in the comment a nickname with a dot sign, for example
Bug 1231810 - Fix files not following flake8 convention ; r?ted.mielczarek
-push it to mozreview


Actual results:

Commit is pushed, but the reviewer request is not done:
"unrecognized reviewer: ted"


Expected results:

The commit should have recognized ted.mielczarek as the reviewer.
(Reporter)

Comment 1

3 years ago
Either a nick including a dot is not a valid nick, or the regex REVIEWER at https://github.com/mozilla/version-control-tools/blob/master/pylib/mozautomation/mozautomation/commitparser.py is not valid.
(Assignee)

Comment 2

3 years ago
a dot is not valid in an irc nick, which is why it isn't present in that regex.

if we were to add it, we'd have to ensure that it doesn't match at the end of the string to avoid "r?glob." from matching "glob." instead of "glob"


ted's :handle on bmo (:ted.mielczarek) doesn't match his irc handle (ted).
of all the users on bmo, only ted (:ted.mielczarek) and bas (:bas.schouten) do this (should be :ted and :bas respectively).

i suspect i may be better to update pylib/mozreview/mozreview/bugzilla/models.py to use the REVIEWER character class from commitparser.py instead of its own BZ_IRCNICK_RE, and fix up and users with periods in their names in the rb database.
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1248196
(Assignee)

Updated

3 years ago
Assignee: nobody → glob
(Assignee)

Comment 4

3 years ago
Created attachment 8719484 [details]
MozReview Request: mozreview: fix pep 8 issues in bugzilla/models.py (bug 1238353) r?smacleod

Fix minor PEP 8 issues.

Review commit: https://reviewboard.mozilla.org/r/34971/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/34971/
Attachment #8719484 - Flags: review?(smacleod)
(Assignee)

Comment 5

3 years ago
Created attachment 8719485 [details]
MozReview Request: mozreview: unify irc nick parsing and disallow periods (bug 1238353) r?smacleod

MozReview and mozautomation's commitparser disagree when it comes to deciding
if a period is valid in an IRC nick.  According to IRC a period is not valid,
which is mozautomation's behavour.  Updating mozreview and rbbz extensions to
use the RE defined in mozautomation instead of duplicating the code.

Review commit: https://reviewboard.mozilla.org/r/34973/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/34973/
Attachment #8719485 - Flags: review?(smacleod)
ted: your MozReview nickname is "ted.mielczarek", which has been... outlawed!  Can you update it and feel the thundering herd land on your review queue?  Thanks!

glob: other MozReview folks: some outreach is needed to move people who have a period in their nick forward.
Flags: needinfo?(ted)
(Assignee)

Comment 7

3 years ago
(In reply to Nick Alexander :nalexander from comment #6)
> glob: other MozReview folks: some outreach is needed to move people who have
> a period in their nick forward.

as per comment 2, only two people are impacted, and i was going to look into fixing their names directly rather than reaching out to them.

ted - don't do anything yet; this code hasn't landed.  if action is required of you, it's likely all you'd have to do is log out then back into mozreview.
(Assignee)

Updated

3 years ago
Flags: needinfo?(ted)
Product: Developer Services → MozReview
Comment on attachment 8719484 [details]
MozReview Request: mozreview: fix pep 8 issues in bugzilla/models.py (bug 1238353) r?smacleod

https://reviewboard.mozilla.org/r/34971/#review32467
Attachment #8719484 - Flags: review?(smacleod) → review+
Comment on attachment 8719485 [details]
MozReview Request: mozreview: unify irc nick parsing and disallow periods (bug 1238353) r?smacleod

https://reviewboard.mozilla.org/r/34973/#review32471

::: pylib/mozautomation/mozautomation/commitparser.py:39
(Diff revision 1)
> +BMO_IRC_NICK_RE = re.compile(':(' + IRC_NICK + ')')

can you stick an `r` prefix on your string literals here, just to stay consistent with the rest of them and incase the regex ever grows.
Attachment #8719485 - Flags: review?(smacleod) → review+
(Assignee)

Comment 10

3 years ago
https://hg.mozilla.org/hgcustom/version-control-tools/rev/894624260e8c
https://hg.mozilla.org/hgcustom/version-control-tools/rev/fc3513aad180

(setting a reminder to fix the accounts once this hits production)
Status: UNCONFIRMED → RESOLVED
Last Resolved: 3 years ago
Flags: needinfo?(glob)
Resolution: --- → FIXED
(Assignee)

Comment 11

3 years ago
forgot to clear this.
Flags: needinfo?(glob)
You need to log in before you can comment on or make changes to this bug.