Page MenuHomePhabricator

Modernize Audit
Closed, ResolvedPublic


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.


  • 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.


  • 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.


  • 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.


D17279 / rP4890d667955b: Excluded authored commits from "Ready to Audit"; handle unreachable commits…
D17276 / rPa3417ccd78d5: Make "bin/audit synchronize" actually save changes
D17271 / rPaca0f642a3bd: Add a "bin/audit synchronize" command
D17268 / rP239b7c7f5c8b: Fix icon spacing for adjacent build status and audit status in commit history…
D17267 / rPbcbd4035fd18: Remove several pieces of audit-related code
D17266 / rP2e9cc5e8e8a3: Make implicit audits by the Owners tool use modern code
D17264 / rP5e7a0917372a: Write an explicit edge for commit membership in packages
D17263 / rP4b248e354587: Make the "Add Auditors" Herald rules use modern transactions
D17262 / rPbc41c3f5a5b9: Use DifferentialCommitMessageParser and Modular Transactions to implement…
D17258 / rP1be3ef022768: Make some Audit status comparisons more lax, so state transactions only post…
D17254 / rP2e3e078358c1: Remove "diffusion.createcomment" Conduit API method
D17253 / rP3b8e2739fca0: Update some Audit documentation
D17225 / rPb8e04fe0419d: Improve handle batching behavior for commit list view
D17194 / rP19525ed81afa: Add Conduit API method
D17191 / rP7d3d0224071a: Restore "[Action]" mail subject lines to Differential/Diffusion
D17190 / rP69d6374646ee: Make new EditEngine Audit transactions apply old mail tags
D17187 / rPb941331bdfe6: Prevent users from resigning from audits they've already resigned from
D17183 / rPb5722a99635a: Use EditEngine stacked comments in Diffusion
D17182 / rP82c891f58663: Add modern "Accept", "Raise Concern" and "Resign" transactions to Audit
D17181 / rP255e3fb1e4bf: Allow auditors to be added and removed from commits in a modern way
D17178 / rP2941b34acbba: Add "diffusion.commit.edit", a v3 edit API endpoint for commits
D17177 / rP279273dc1c93: Replace old commit edit controller with new EditEngine controller
D17176 / rP5e073588268c: Preserve "Autoclose?" information on new Commit edit flow
D17175 / rP7ff0be1bde2f: Bring very basic EditEngine support to commits

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
epriestley updated the task description. (Show Details)May 17 2016, 2:41 PM
alexnb added a subscriber: alexnb.May 20 2016, 8:22 AM
knot added a subscriber: knot.Jun 23 2016, 7:08 PM
adam93 added a subscriber: adam93.Nov 1 2016, 2:03 PM
nornagon added a subscriber: nornagon.
arr_sea added a subscriber: arr_sea.Dec 7 2016, 1:58 AM
epriestley moved this task from Backlog to EditEngine on the Audit board.Jan 10 2017, 4:37 PM
monsdar removed a subscriber: monsdar.Jan 11 2017, 6:39 PM
joshma added a subscriber: joshma.Jan 15 2017, 12:17 AM
epriestley closed this task as Resolved.Feb 1 2017, 3:05 AM
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.