This is intentional, but maybe should work differently than it does.

Unlike review, we don't have any kind of "Needs Re-audit" state, so after the commit author fixes the concern they can't push the commit back to the auditor through state changes. We could add such a state, but it felt very heavy for v1, and hasn't been requested or felt necessary since then (for our workflow, at least).

Since there's no "Needs Re-Audit" state, we just keep the "Concern Raised" audits in "Need Attention", because the next state change is an "Accept" after the author fixes the issue, which is an action you take. (Since audit is nonblocking, the tentative-next-action is also "hassle the author to fix the problem".)

We could add a "Needs Re-Audit" state (maybe "Needs Verification"), an author action to push audits into that state (maybe "Request Verification"), and a table for audits you've raised concern with (maybe "Waiting on Others"). Would this work better for your workflow?

(The specific problem with the snippet in the description is that those commits are now no longer on your radar, but you need to take the next action on them -- moving them to "Accepted" once the problem is fixed [and/or hassling the author before then].)

We could add a "Needs Re-Audit" state (maybe "Needs Verification"), an author action to push audits into that state (maybe "Request Verification"), and a table for audits you've raised concern with (maybe "Waiting on Others"). Would this work better for your workflow?

That would work really well.

Another problem is there's no way to tie two commits together like there is to tie two diffs into one revision. This makes the "I've addressed this problem" workflow really awkward since there's no phabricator metadata attached to the two commits.

The audit workflow right now is:

New developer: I submit commit v1
Experienced pro: Raise concern with commit v1.
New developer: I submit commit v2. This commit addresses v1 concern.
Experienced pro: ????

The ??? is the part that I'm lost at. Phabricator could tie v1 and v2 with some metadata that has them referencing each other from the Audit view. Ideally, there would be something in the git commit template that is like:

Addresses Commit:

and it would put the addressed commit back in Request Verification state after I "git push". This "Addresses Commit" edge creates a graph of commits. (commit v3 may address commit v2 and commit v4 may address a problem in commit v1. However, v1/v2/v3/v4 can be seem as the same problem to solve). That kind of sounds complicated, but I think it mirrors the time tested differential revision/diff workflow well.

I'm hacking this "Show the connected graph of commits on the Audit page" thing together right now with the "edge" abstraction in phabricator, but don't know if you have better ideas or what the whole "???" workflow would ideally be.

An "Addresses Commit" field (and maybe automatic pattern matching in commit summaries like we have for "Fixes T123", "Closes T123", etc.) seems completely reasonable -- the only reason we don't have it is that it needs the new state stuff to really be very meaningful (i.e., automatically take action rather than just showing data in the UI), and I didn't want to build that in Audit-v1. Edges are appropriate to store this data.

◀ Merged tasks: T2810.

I think T2810 is proposing an essentially identical mechanism (correct me if I misread). Definitely reasonable.

Via FB (via T2810).

I'd really like to see this feature!

We just started using phabricator and the only thing we "miss" is something to do this:

• Dev 1 commits something (r1234)
• Dev 2 raises a concern
• A few days later Dev 1 adresses these concerns and commits updated code (r1240)
• Now the '1240' should be "linked" to '1234' so dev 1 knows his problems are adressed and he can now "accept" the comment.

I asked on IRC about this and epriestley told me about this issue.

TL;DR

I'd really like to have a way to add a 'needs verification' to a commit, something like: "Addressess commit: r1234"

At company I'm working at we have workflow, where people must prefix 1st commit message line with reference to fixed commit like so: [fixes: rABC434353] (space at the end).

Then thanks to fact, that Phabricator since recently collects mentions of tasks in another task comments I can see on original commit where it is fixed.

Since I'm auditing all commits the [fixes ...] prefix clearly separates fix commits from others, which also helpful.

Anything done on this? I'd like to request a pull to work on this.
I am referring to @matthijsmvs feature request:

I'd really like to have a way to add a 'needs verification' to a commit, something like: "Addressess commit: r1234"

After D17252, audits now have a "Request Verification" action and a "Needs Verification" state. These states prompt auditors to verify the remedies the author has offered have addressed their concerns. Intended usage is along these lines:

An auditor (@alice) identifies a concern with a commit:

alice raised a concern with this commit.
"Once this went into production, we saw a fatal when X = 3. Can you fix that for the next release?"

This moves the commit to "Concern Raised" and puts it back in the action queue for the author (@bailey). The author addresses the concern, then requests verification:

bailey mentioned this in rXYZaaaa.
bailey requested verification of this commit.
"The X = 3 issue should now be fixed in rXYZaaaa, which is headed to production momentarily."

This moves the commit to "Needs Verification", and returns it to the auditor's queue. They can accept it to complete the audit:

alice accepted this commit.
"Thanks, looks good now."

They can also raise another concern if things haven't been fully addressed, repeating the process.

The original task description discusses adding a "Remedies" field, like "Remedies: rXYZaaaa". While we could still do this, I'd like to see if it's really necessary in the context of modern Phabricator. In particular, mentions are now more ubiquitous (as in my example above). I also think it's good to put a mild onus on the author to explain to the reviewers why they believe concerns are now addressed and provide relevant context.

If we do pursue this, it would probably be in the context of explicitly relating reverts (T1751, etc), since that's the stronger relationship between commits.

If you use this for a bit and are convinced you want a "Remedies <commit>" sort of magic word, feel free to file a new task.

I also don't love the language here, and think it could probably be clearer, since "Needs Verification" isn't totally clear as "Needs Authors to Verify that This is Fixed". There are enough contextual clues that this is probably self-evident and I'll update the documentation, but if anyone has clearer language I'm open to it.