As a reviewer, I want to use comments and diffs on a pull request to clearly track iterative changes made during the review of a pull request.

I would like to point out a need that I have found while performing reviews via stash. A similar need exists when using Crucible as well, but I want to specifically address it as it relates to Stash.

The typical workflow for reviewing a pull request in Stash is this.

• Engineer submits a pull request
• Reviewer looks at the diff and adds comments
• Engineer looks at comments and makes changes
• Engineer pushes their changes
• Reviewer looks at the global diff in stash and reviews the code again

This process repeats itself until the reviewer is happy with the diff in stash, at which point they can approve and accept the pull request.

The problem with this workflow is that as reviews get larger, and as the number of iterations grow more numerous, that the burden is on the reviewer to make sure their comments and concerns were addressed by subsequent commits. Because the diff in stash encompasses the entire pull request, or the diff between that branch and the target branch, there is no way to see iterative diffs alongside the comment view. That basically means that the reviewer has two options for how to re-review the changes once the engineer commits again:

1) Treat the latest commit as a new pull request and re-review the entire set of changes. The benefit of this is supreme confidence that high quality code will make it into master. The downside is this greatly increases the amount of time that the reviewer must spend reviewing code because they are forced to also re-review code that they implicitly "green-lighted" on the previous review by not commenting on.

2) Use a combination of the Overview activity list to map their comments to the individual commit diff for each new commit since their last review. The individual commit diffs do show the iterative progress that the reviewer needs to see so that they know what new changes were made. But those diffs do not include comments, so they also need to review their own comments in a separate window to remind them of the context of the changes and the rest of the pull request.

Neither of these two things are major issues for small, targeted pull requests. I'll concede, for the sake of argument, that small, targeted, pull requests are ideal and a best practice. But I think we can still agree that this doesn't always happen, and I think the tool can support a broader and more complete workflow by improving the user experience for the scenario I describe above.

So, here's my proposed solution. I think the tool should support one or both of the following features:

1) On the Diff view in a pull request, allow the user to view either the global pull request diff, from first commit to last commit, or specify specific revisions to view the diff from and to. That will keep the Diff view as the single source of truth, while also improving the review workflow for iterative reviews.

2) When the user clicks on a commit link from the pull request overview activity list, instead of taking the user to view the diff for that commit in isolation, pull in the comments to also show those there.

I am really interested to hear what the Stash team thinks about this proposed change. While we like Stash, and we see it as an improvement to Crucible for our workflow, I think these changes would take it to a whole new level for us by streamlining the iterative code review workflow for when a review takes several iterations to get right.

Thanks,

• Conrad