Closed Bug 1238353 Opened 7 years ago Closed 7 years ago
specifying reviewer with dot sign in the nickname doesn't work
58 bytes, text/x-review-board-request
58 bytes, text/x-review-board-request
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 https://github.com/mozilla/version-control-tools/blob/master/pylib/mozautomation/mozautomation/commitparser.py 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/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.
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)
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.
(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.
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+
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
Closed: 7 years ago
Resolution: --- → FIXED
forgot to clear this.
You need to log in before you can comment on or make changes to this bug.