Page MenuHomePhabricator

Flagging commits via Herald no longer generates flag note
Open, LowPublic

Description

I have personal Herald rule setup, that flags all commits that touch certain files. Before last update the created flag had Flagged by Herald Rule "{Herald Rule Name}". note. Right now note is empty.

The change was made as part of T8726 task, but I'm not sure if removing flag note was done on purpose (task doesn't mention that) or by accident.

Related commit: rP8d9bd791f7f263648c5a811695a000889a136be7

Event Timeline

aik099 created this task.Aug 12 2015, 3:19 PM
aik099 raised the priority of this task from to Needs Triage.
aik099 updated the task description. (Show Details)
aik099 added projects: Herald, Flags.
aik099 added a subscriber: aik099.
aik099 updated the task description. (Show Details)Aug 12 2015, 3:40 PM
epriestley moved this task from Backlog to vNext on the Flags board.Jan 6 2016, 3:18 AM
epriestley triaged this task as Low priority.Jan 6 2016, 3:21 AM
epriestley added subscribers: lpriestley, epriestley.

I removed this because the implementation was pretty hacky (saved the rule as a text string, wasn't translatable).

To restore it, we should do this:

  • Add a reasonPHID or similar to PhabricatorFlag. Normally this is null.
  • When Herald adds a flag, store the Herald rule PHID in reasonPHID.
  • In the UI, show information about why a flag was added if reasonPHID is not null (e.g., "Added by H23 Some Rule Name").

This is translatable, will still work correctly if the rule name changes, can link to the rule, and could be searched for in the future (though I don't think we need to implement this now).

eadler added a project: Restricted Project.Apr 11 2016, 9:41 PM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Apr 17 2016, 6:24 PM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Jul 4 2016, 9:18 PM
epriestley moved this task from Backlog to Future on the Herald board.Jan 26 2018, 5:51 PM