`r!foo,bar` that gets turned into `r=foo!,bar` instead of `r=foo!,bar!`
Categories
(Conduit :: moz-phab, defect, P2)
Tracking
(Not tracked)
People
(Reporter: kats, Assigned: glob)
Details
(Keywords: conduit-triaged)
Attachments
(1 file)
With https://phabricator.services.mozilla.com/D27598 I initially had the commit message ending with r!botond. That didn't work in that it left the patch with no reviewers. Then I edited the commit locally to be r?botond! and resubmitted. Again it did not add any reviewers, so I added botond manually using the web UI.
According to Kwan on IRC the r?botond! syntax should have worked. Maybe it doesn't work if I edit locally and repush?
Regardless it would be good if the syntax was actually documented somwhere. I searched in https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html and didn't see anything, and the moz-phab submit -h command just says that you can use --blocker to override the default ("from commit") but doesn't say what syntax is needed in the commit message.
i'm unable to reproduce this with the latest version of moz-phab.
do you still have a copy of the moz-phab output from your initial submission?
how the r!botond support works is it rewrites that to r=botond! prior to submitting to phabricator.
this rewriting is visible in the list of commits prior to the confirmation prompt.
until bug 1545158 is fixed (thanks for filing!) you can check if your version of moz-phab supports the r!botond syntax by seeing if it has a def morph_blocking_reviewers(): line.
According to Kwan on IRC the r?botond! syntax should have worked. Maybe it doesn't work if I edit locally and repush?
that's bug 1501077.
|
Reporter | |
Comment 2•7 years ago
|
||
My moz-phab doesn't have the morph_blocking_reviewers function, so I guess it's too old. Timestamp on the file is from March 4th.
|
||
Comment 3•7 years ago
|
||
Please run moz-phab self-update and try again.
Does the problem still exist?
|
|
Reporter | |
Comment 4•7 years ago
|
||
It works after updating moz-phab, thanks! Perhaps this bug should just be about documenting this properly somewhere, then.
|
|
Reporter | |
Comment 5•7 years ago
|
||
Also, if I do r!foo,bar that gets turned into r=foo!,bar instead of r=foo!,bar! like I would expect.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4)
Perhaps this bug should just be about documenting this properly somewhere, then.
this bug is to fix a common mistake rather than making it a supported syntax.
i'd prefer to leave it undocumented.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5)
Also, if I do
r!foo,barthat gets turned intor=foo!,barinstead ofr=foo!,bar!like I would expect.
oops - let's morph this bug to that issue :)
|
|
Reporter | |
Comment 7•7 years ago
|
||
(In reply to Byron Jones ‹:glob› 🎈 from comment #6)
this bug is to fix a common mistake rather than making it a supported syntax.
i'd prefer to leave it undocumented.
When I said this should be documented, I was referring to the syntax in general. I don't care whether you recommend r=foo! or r!foo as long the syntax is documented somewhere, which AFAICT it is not currently.
it's on the faq under https://wiki.mozilla.org/Phabricator/FAQ#How_do_I_require_a_review_from_all_reviewers_before_landing.3F but i agree it isn't clear at all (i initially thought it wasn't there!). i'll update the faq.
|
|
Reporter | |
Comment 9•7 years ago
|
||
Oh dear. Can we merge the moz-conduit.readthedocs.io documentation with the wikimo documentation? I see the thing that says "The FAQ is on a wiki to make it easier to maintain;" but this is somewhat user-unfriendly because you can't just ctrl-f the docs to find what you're looking for.
|
Assignee | |
Comment 10•7 years ago
|
||
|
|
Assignee | |
Comment 11•7 years ago
|
||
this has landed.
Description
•