Page MenuHomePhabricator

Allow monogrammed objects to be parsed from the `arc` command line in "Reviewers" and similar fields
ClosedPublic

Authored by epriestley on May 13 2016, 3:14 PM.
Tags
None
Referenced Files
F14356979: D15911.id38327.diff
Fri, Dec 20, 1:52 AM
F14356978: D15911.id38315.diff
Fri, Dec 20, 1:52 AM
F14356970: D15911.id.diff
Fri, Dec 20, 1:51 AM
F14356838: D15911.diff
Fri, Dec 20, 12:40 AM
Unknown Object (File)
Wed, Dec 18, 10:56 AM
Unknown Object (File)
Thu, Dec 12, 10:33 PM
Unknown Object (File)
Thu, Dec 12, 10:33 PM
Unknown Object (File)
Thu, Dec 12, 10:33 PM
Subscribers
None

Details

Summary

Ref T10939. This allows the CLI to parse reviewers and subscribers like this:

Reviewers: epriestley, O123 Some Package Name

The rule goes:

  • If a reviewer or subscriber starts with a monogram (like X111), just look that up and ignore everything until the next comma.
  • Otherwise, split it on spaces and look up each part.

This means that these are valid:

alincoln htaft
alincoln, htaft
#a #b epriestley
O123 Some Package, epriestley, #b

I think the only real downside is that this:

O123 Some Package epriestley

...ignores the "epriestley" part. However, I don't expect users to be typing package monograms manually -- they just need to be representable by arc land and arc diff --edit and such. Those flows will always add commas and make the parse unambiguous.

Test Plan
  • Added test coverage.
  • amend --show'd a revision with a package subscriber (this isn't currently possible to produce using the web UI, it came from a future change) and saw Subscribers: O123 package name, usera, userb.
  • Updated a revision with a package subscriber.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Allow monogrammed objects to be parsed from the `arc` command line in "Reviewers" and similar fields.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.

This technically already supports listing them as O123 without any label, and we do that for Maniphest Tasks: T123. We could stick with that here, but it seems worthwhile to provide a more human-readable label here because Reviewers: O123, epriestley is pretty hard to work with from the CLI when you have no clue what O123 is, and I think there's more need to work with reviewers from the CLI than to work with tasks from the CLI.

chad edited edge metadata.
This revision is now accepted and ready to land.May 13 2016, 7:11 PM
This revision was automatically updated to reflect the committed changes.