Page MenuHomePhabricator
Create Task
Maniphest T13041

In Herald Commit rules, "Reviewer" and "On Autoclose Branch" have been deprecated
Open, WishlistPublic
Actions

Description

See PHI301.

For Commit rules in Herald, the Commit Fields → Reviewer field evaluates to only the first reviewer for the associated revision. This is legacy behavior from an era (pre T1279) where revisions broadly had only one reviewer.

This doesn't match up with modern behavior very well, and modern users almost certainly want Related Fields → Accepting reviewers instead. The "Reviewer" field is probably never useful, and potentially confusing.

It's not entirely clear how to best move forward:

  • Remove the field -- but we break existing rules.
  • Migrate the field to "Accepting reviewers" -- but we change the behavior of existing rules.
  • Rename the field to "Reviewer (Legacy)", or move it to a "Legacy" category -- maybe improves things, but really just punts this.

Revisions and Commits

rP Phabricator
D18932rPa9f87857af4b Mark the "Reviewer" field for Commits as deprecated

Related Objects

Event Timeline

epriestley triaged this task as Low priority.Jan 18 2018, 2:58 PM
epriestley created this task.
Herald added a subscriber: eadler. · View Herald TranscriptJan 18 2018, 2:58 PM
epriestley mentioned this in T13048: Implement a "take action only when this rule didn't match last time" repetition policy for Herald.Jan 25 2018, 6:20 PM
epriestley added a revision: D18932: Mark the "Reviewer" field for Commits as deprecated.Jan 25 2018, 6:28 PM
epriestley lowered the priority of this task from Low to Wishlist.Jan 25 2018, 6:29 PM

Rename the field to "Reviewer (Legacy)", or move it to a "Legacy" category -- maybe improves things, but really just punts this.

D18932 does this, to hopefully reduce confusion at least.

epriestley moved this task from Backlog to Far Future on the Herald board.Jan 26 2018, 5:50 PM
epriestley added a commit: rPa9f87857af4b: Mark the "Reviewer" field for Commits as deprecated.Jan 26 2018, 7:07 PM
epriestley moved this task from Backlog to Far Future Work on the Differential board.Feb 10 2018, 7:40 PM
epriestley moved this task from Backlog to "Track Only" on the Diffusion board.Apr 15 2019, 3:34 PM
Herald added a subscriber: amckinley. · View Herald TranscriptApr 15 2019, 3:34 PM
epriestley added a comment.Apr 15 2019, 5:05 PM

D20427 is a similar change, that deprecates the "Commit is on autoclose branch" field. This fields is now always true by definition, because Herald does not run until a commit is reachable from a permanent ref (previously called an "autoclose branch").

epriestley renamed this task from In Herald Commit rules, "Commit Fields > Reviewer" has ancient, misleading behavior to In Herald Commit rules, "Reviewer" and "On Autoclose Branch" have been deprecated.Apr 15 2019, 5:08 PM
epriestley removed a project: Differential.
epriestley mentioned this in D20427: Deprecate the "Commit is on autoclose/permanent branch" Herald "Commit" field.Apr 17 2019, 8:14 PM
epriestley mentioned this in T13299: Herald only triggers for commits once, during the initial publish phase.May 27 2019, 3:34 PM
Phabricator · Built by Phacility