Closed Bug 1238353 Opened 9 years ago Closed 8 years ago

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


(MozReview Graveyard :: General, defect)

Not set


(Not tracked)



(Reporter: simon, Assigned: glob)




(2 files)

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.
Either a nick including a dot is not a valid nick, or the regex REVIEWER at is not valid.
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/ to use the REVIEWER character class from instead of its own BZ_IRCNICK_RE, and fix up and users with periods in their names in the rb database.
Assignee: nobody → glob
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:
See other reviews:
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)
(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.
Flags: needinfo?(ted)
Product: Developer Services → MozReview
Comment on attachment 8719484 [details]
MozReview Request: mozreview: fix pep 8 issues in bugzilla/ (bug 1238353) r?smacleod
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

::: pylib/mozautomation/mozautomation/
(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+

(setting a reminder to fix the accounts once this hits production)
Closed: 8 years ago
Flags: needinfo?(glob)
Resolution: --- → FIXED
forgot to clear this.
Flags: needinfo?(glob)
You need to log in before you can comment on or make changes to this bug.