Page MenuHomePhabricator

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

Description

See PHI301.

For Commit rules in Herald, the Commit FieldsReviewer 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 FieldsAccepting 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.

Event Timeline

epriestley triaged this task as Low priority.
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 moved this task from Backlog to "Track Only" on the Diffusion board.Mon, Apr 15, 3:34 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.Mon, Apr 15, 5:08 PM
epriestley removed a project: Differential.