Page MenuHomePhabricator

Herald Rule to assign Owners to an Audit
Closed, WontfixPublic

Description

Prior to recent changes apparently motivated by T10939, assigning Owners was an easy way to configure 100% code audit for an area of code.

After those changes, any commits by an Owner are bypassing the audit. To get back the old behaviour, it is now necessary to duplicate all the Owners configuration as Herald rules.

This could be made easier if Herald had an Action on commits to "Add Owners as Auditors". The intention of this action would be to unconditionally add Owners as Auditors - even when the author of the change is an owner. With such a rule, a single Herald rule would be all that is needed to restore the old behaviour of raising audits on all changes.

Event Timeline

If the "Audit" setting for packages had three settings instead, would that solve your use case?

Audit: Off
Audit: Commits not authored or reviewed by a package owner
Audit: All Commits

Yes, that would also solve the problem for me, and would probably be an easier option for new users.

I suggested the Herald solution, as it might be more flexible for other's needs, and the decision to change the Owners behaviour seems to have been deliberate, so I wasn't expecting much motivation for changing it there. But ultimately it is your decision how/if/when to fix it.

On second thoughts, the flexibility of a Herald rule may already be better for me, as I want to filter out Merge Commits from auditing (at least when they are merged by an Owner), since the individual commits that are merged already have their own audit.

Again though, this may be a universal thing that most users will want so it may be better handled by always filtering those out, though I imagine there are some users working in an environment that is so tied down in process that they need to show that they are auditing every single change, even when the change is just a merge of other already audited changes.

We're looking for a similar concept for Reviewers:
Even if the Author of a review is one of the Owners, we still want to automatically add that package.
This is mostly just T1125 - we want to make it easier to assign reviewers, and Owners has good UI for selecting paths.

Using Herald for this just ends up with extra work (Write another rule for each package).

eadler added a project: Restricted Project.Sep 15 2016, 6:08 PM
epriestley claimed this task.

Owners review and auditing now have 6 and 4 options respectively, which I think cover most of the needs here. They don't handle everything (e.g. excluding merge commits) but think we're mostly in a reasonable place now and don't have any current plans to add additional shorthands.