Some possible approaches:
I was just looking for something like this.
I think reviewboard issues does a great job of making it easy to go back and see what I have left to address on a diff, and for reviewers to see that the important things have indeed been taken care of or at least considered and closed out.
I'd be willing to put in some time implementing this if there's a straightforward way to add this.
This was prompted by me thinking about how to measure the effectiveness of code reviews, then realizing that not only is that difficult with the free-form approach Differential takes now, but it actually makes it harder for everybody involved to figure out what's going on. This is particularly true on hairy patches with lots of back and forth in the review cycle.
I think T1460 might be a closer match to the reviewboard feature (basically, giving inline comments a lightweight "done/replied" state and keeping track of them in the UI). I think it's mostly ready to implement, although there may be some remaining infrastructure work.
I think the driving case here is more like: something comes up in discussion, everyone agrees that it should be fixed later but doesn't need to be fixed now, and there's some sort of "File a Task" button which makes it easier to persist the concern beyond the scope of the review (currently, we'd normally deal with this by putting a "TODO" comment in the file, which gets picked up by lint).
In general, I think hairiness and back-and-forth is often a symptom of patches being too large and too complex. We can provide somewhat better tools, but the most effective approach by far will always be to break large ideas down into small ones and produce smaller, simpler patches.
There are a few features -- one example is keeping track of which files you've looked at and saying "this review is 80% complete because you've looked at 80% of the files" -- which get proposed every so often that I think only really help with too-complex reviews, and are fairly moot or even harmful on reviews of appropriate complexity. Generally, the most valuable feedback on reviews by far is high level feedback about the big picture: does the change make sense?. My quip here is that the most valuable piece of feedback possible on a review is "don't commit this". This is very hard to measure, and will probably never show up on metrics about how many tasks were filed as a result of review, but stopping a change which doesn't make sense (for product or technical or roadmap or security or other reasons) from continuing can save an enormous amount of time. Because I think the big picture is so much more important than the details, I'm hesitant to implement features which focus mostly on managing details, and particularly features which impose very rigid, detail-oriented structure on code review (like the notion that code review is complete when you've looked at every line: I think this is poisonous thinking, and misses the forest for the trees).
I think we can do done/replied in a way that doesn't focus on details too much (T1460), and I think "File a Task" is a reasonable approach for some issues too (it mostly just doesn't fit easily into the UI right now), but want to balance these features carefully in cases where they make structure more rigid.
I also don't know how to measure the effectiveness of code review objectively, but I think looking at comments and resolution is probably not a strong signal. One example is that the introduction of a robust lint pipeline at Facebook essentially removed the need for anyone to ever leave a style/spacing/naming inline again and saved a huge amount of time and made review far more pleasant, but would have looked like a step backward in terms of a "number of things caught in review" metric.
I agree with everything you said here :)
A lot of the "back and forth" reviews we do start as a POC that evolves into actual shipping code. A somewhat more efficient version of pair-programming that allows all discussion (from high level, how should we structure this to tiny corner cases) to be recorded on one diff. In those situations in particular, it's helpful to have a "Done" button as tiny things caught early in the review cycle might not get fixed due to general restructuring that's happening, then actually slip through the cracks or annoy the reviewer that their little details weren't also fixed.
As far as measuring the effectiveness of code review, I wouldn't want to be measuring "number of things caught in review" for a particular engineer necessarily, but being able to see that installing a robust lint pipeline had x% increase in efficiency as measured by issues filed against a review would not be a useless signal (IMO) and would help justify the engineering time on that lint pipeline.
As another example, I was thinking of pushing on people to first review their own reviews (some people do this voluntarily, but most don't). It would be cool to be able to point at a metric and say "hey, people who do this have x% fewer issues on their diffs, which means you're shipping better code and reviewing better code"
Cool, sounds like we're pretty much on the same page. I think T1460 is more-or-less ready to go, we did eventually land enough of the storage change that they don't need to block anything.
Here are the basic use cases that I had been thinking about when I bugged @epriestley about this a long time ago:
The manual workaround was simply to open a task and paste its task number into a follow up comment. Most often this occurred with dealing with legacy code.
My current job doesn't use phabricator, so I no longer have a dog in this fight.
In the upstream, for things that are specific, we generally leave TODO comments in the code itself (and a reviewer who catches something will suggest leaving a TODO). This has some advantages. For one, you can see comments in the code itself. And, with a linter, you can extract TODO comments and track them, see:
I generally think this avenue is the more promising avenue, and would broadly prefer to pursue making it easier to raise issues directly from source code over strengthening workflows which establish a tenuous link between source and issues.
(In the upstream, for things that are general, we just file a task. T7642 is an example. My experience is that most of these are somewhat general and not tightly tied to a specific piece of code or revision. Most issues with a specific revision should be getting resolved before it lands.)