Page MenuHomePhabricator

Modernize Audit
Closed, ResolvedPublic

Description

Long ago, Audit was copy/pasted from Differential and then essentially never changed again.

Audit generally suffers from being un-modern and playing second fiddle to Differential.

Workflow

  • When an author has addressed concerns, there is no way for them to punt the audit back into the auditors' queues (T2393). Generally, this workflow is currently a dead end.
  • The UI offers "Accept Commit" even if it's meaningless and not permissible (T5928). Maybe a broader workflow issue.
  • Users should be able to add and remove auditors (T7676), like they can add and remove reviewers in Differential.
  • T7504 reports a bug with manually added auditors and overall audit status.

Herald

  • T5889 wants an "Auditors" Herald field, similar to the Differential "Reviewers" field.

Search and Status

  • Search should have bucketing, like Differential (T9430). Generally, the UI should be aligned with the modern Differential UI.
  • Search should support a modern status selector (T9544).
    • Possibly, "Audits Resolved" should be different from "No Audits Triggered" (T8683).
  • T6024 wants audit status shown in Diffusion commit lists, similar to how build status is shown.
    • Status behavior elsewhere is likely ripe for resolution now (T9482).
    • T2921 reports a bug with this but is extremely old.
  • T6660 is a request for a draft icon in the list view, like the one in Differential.

Infrastructure

  • This stuff should all work via EditEngine.
  • Audit requests store untranslatable descriptive text in the database, but should store translatable reason codes.
  • Good news: we never moved audit requests to edges, so we don't have to resolve an analog of T10967.

See also T10685, T9091, T4713, which overlap or cover multiple issues and which I've merged here.

Revisions and Commits

rP Phabricator
D17279
D17276
D17271
D17268
D17267
D17266
D17264
D17263
D17262
D17258
D17254
D17253
D17225
D17194
D17191
D17190
D17187
D17183
D17182
D17181
D17178
D17177
D17176
D17175

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
epriestley claimed this task.

I think I've pushed this approximately as far as I plan to in this iteration. Everything mentioned in the description has been tackled, and audits on this install are now in a relatively sane state.

The workflow has changed a little and I'm sure there are some remaining rough edges; feel free to file followups as you run into them.

I plan to continue broader work on Diffusion as described in T12010.