This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6512

phabricator: add --blocker argument to phabsend to specify blocking reviewers
ClosedPublic

Authored by Kwan on Jun 11 2019, 2:28 PM.

Details

Reviewers
durin42
mharbison72
Group Reviewers
hg-reviewers
Commits
rHGf33d3ee110da: phabricator: add --blocker argument to phabsend to specify blocking reviewers
Summary

The way to signal to Conduit that a reviewer is considered blocking is just to
wrap their PHID in "blocking()" when including it in the list of PHIDs passed
to reviewers.add.

arc doesn't have a --blocker, instead one is supposed to append a '!' to the
end of reviewer names (I think reviewers are usually added in an editor rather
than the command line, where '!'s can be more hazardous).

moz-phab (Mozilla's arcanist wrapper) does have a --blocker argument, and being
explicit like this is also more discoverable. Even arc diff's help doesn't
seem to mention the reviewer! syntax.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Kwan created this revision.Jun 11 2019, 2:28 PM
Kwan retitled this revision from phabricator: add a --blocker argument to phabsend to specify blocking reviewers to phabricator: add --blocker argument to phabsend to specify blocking reviewers.Jun 11 2019, 2:55 PM
mharbison72 accepted this revision.Jun 12 2019, 12:37 PM

LGTM. Thanks for doing this.

Kwan added a commit: rHGf33d3ee110da: phabricator: add --blocker argument to phabsend to specify blocking reviewers.Jun 12 2019, 12:44 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGf33d3ee110da: phabricator: add --blocker argument to phabsend to specify blocking reviewers (authored by Kwan). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/phabricator.py (10 lines)

Diff 15454

hgext/phabricator.py

raise error.Abort(_(b'unknown username: %s') raise error.Abort(_(b'unknown username: %s')
% b' '.join(sorted(unresolved))) % b' '.join(sorted(unresolved)))
return [entry[b'phid'] for entry in data] return [entry[b'phid'] for entry in data]
@vcrcommand(b'phabsend', @vcrcommand(b'phabsend',
[(b'r', b'rev', [], _(b'revisions to send'), _(b'REV')), [(b'r', b'rev', [], _(b'revisions to send'), _(b'REV')),
(b'', b'amend', True, _(b'update commit messages')), (b'', b'amend', True, _(b'update commit messages')),
(b'', b'reviewer', [], _(b'specify reviewers')), (b'', b'reviewer', [], _(b'specify reviewers')),
(b'', b'blocker', [], _(b'specify blocking reviewers')),
(b'm', b'comment', b'', (b'm', b'comment', b'',
_(b'add a comment to Revisions with new/updated Diffs')), _(b'add a comment to Revisions with new/updated Diffs')),
(b'', b'confirm', None, _(b'ask for confirmation before sending'))], (b'', b'confirm', None, _(b'ask for confirmation before sending'))],
_(b'REV [OPTIONS]'), _(b'REV [OPTIONS]'),
helpcategory=command.CATEGORY_IMPORT_EXPORT) helpcategory=command.CATEGORY_IMPORT_EXPORT)
def phabsend(ui, repo, *revs, **opts): def phabsend(ui, repo, *revs, **opts):
"""upload changesets to Phabricator """upload changesets to Phabricator
confirm |= bool(opts.get(b'confirm')) confirm |= bool(opts.get(b'confirm'))
if confirm: if confirm:
confirmed = _confirmbeforesend(repo, revs, oldmap) confirmed = _confirmbeforesend(repo, revs, oldmap)
if not confirmed: if not confirmed:
raise error.Abort(_(b'phabsend cancelled')) raise error.Abort(_(b'phabsend cancelled'))
actions = [] actions = []
reviewers = opts.get(b'reviewer', []) reviewers = opts.get(b'reviewer', [])
blockers = opts.get(b'blocker', [])
phids = []
if reviewers: if reviewers:
phids = userphids(repo, reviewers) phids.extend(userphids(repo, reviewers))
if blockers:
phids.extend(map(
lambda phid: b'blocking(%s)' % phid, userphids(repo, blockers)
))
if phids:
actions.append({b'type': b'reviewers.add', b'value': phids}) actions.append({b'type': b'reviewers.add', b'value': phids})
drevids = [] # [int] drevids = [] # [int]
diffmap = {} # {newnode: diff} diffmap = {} # {newnode: diff}
# Send patches one by one so we know their Differential Revision IDs and # Send patches one by one so we know their Differential Revision IDs and
# can provide dependency relationship # can provide dependency relationship
lastrevid = None lastrevid = None