On 02/04/16 21:59, Gregory Szorc wrote:
When you say "I almost never want to review individual commits and instead want to review the changeset as a single diff," I'm confused because a commit is a changeset (in Mercurial terms at least) and this statement is contradictory. You seem to be saying that you want to look at a series of changesets/commits as a single diff? (I'm guessing your definition of "changeset" doesn't match mine.) Anyway, you seem to be advocating for the GitHub model where there are N commits on a pull/review request but most people typically only look at the aggregate/final diff.
FWIW it's my experience that the GitHub model is that people --amend every commit and review using the built-in tools is almost impossible because history keeps getting lost. That is just a deficiency of GitHub, of course, but it means that your comparison doesn't make much sense to me.
The widely practiced Firefox code review model is to try to ensure that commits that reviewers see - whether on Bugzilla or MozReview - are as close to their final, landing state as possible. In my opinion, the model of submitting all the original, intermediate, to-be-thrown-away commits adds all kinds of UI/UX challenges, overhead, and dissonance to the code review process. It is much simpler for reviewers and the tooling to require history rewriting occur on the client side and for the proposed, final commits - and only the proposed, final commits - to be the thing exposed to MozReview.
This seems like a mostly orthogonal issue to wanting to see the effect of squashing multiple commits. A patch author might have chosen to break their change into a number of small commits that build individually, yet a reviewer may want to see how they fit together into a coherent whole. Without this it's possible to end up in a "can't see the forest for the trees" situation, where each change looks OK on its own, but the overall patch series is missing an important element. Noticing what *isn't* present is much harder than verifying what is, and so we should make tools that can help reviewers as much as possible.
_______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
