On Jul 18, 2012, at 9:35 PM, Roan Kattouw wrote: > On Wed, Jul 18, 2012 at 9:30 PM, Subramanya Sastry > <ssas...@wikimedia.org> wrote: >> (b) Commit amends hide evolution of an idea and the tradeoffs and >> considerations that went into development of something -- the reviews are >> all separate from git commit history. All that is seen is the final >> amended commit -- the discussion and the arguments even that inform the >> commit are lost. Instead, if as in (a), a topic branch was explicitly (or >> automatically created when a reviewer rejects a commit), and fixes are >> additional commits, then, review documentation could be part of the commit >> history of git and shows the considerations that went into developing >> something and the evolution of an idea. >> >> There was an email recently on wikitext-l where Mark Bergsma was asked to >> squash his commits (http://osdir.com/ml/general/2012-07/msg20847.html) -- I >> personally think it is a bad idea that is a result of the gerrit review >> model. In a different review model, a suggestion like this would not be >> made. >> > > Although squashing and amending has downsides, there is also an > advantage: now that Jenkins is set up properly for mediawiki/core.git > , we will never put commits in master that don't pass the tests. With > the fixup commit model, intermediate commits often won't pass the > tests or even lint, which leads to false positives in git bisect and > makes things like rolling back deployments harder. > > Roan >
I disagree. Contrary to what many think, every single patch set and amendment goes into the mediawiki/core.git repository, whether reviewed and passing, or a fresh mistake. This is easily verified by the fact that every patch set revision has its own gitweb link, and the fact that git-review downloads the merge request from a remote branch, inside the core.git repo (or whatever the repo may be). git-bisect is not an issue now and won't be an issue in the branch-review model[1], because git-bisect only affects the HEAD's tree (naturally). The way to merge a branch would be to squash the branch into one commit when merging (e.g. the "merge" commit). This is also how jQuery lands branches most of the time, and iirc GitHub (as in, the internal repo github.com/github/github) also works this way with long-lived branches (based on a presentation they gave; "How GitHub uses GitHub to build GitHub"[2]). And, of course, we will stick to the model of only allowing merges when tests pass. So the HEAD's history will remain free of commits that don't pass tests. One could argue that then the branches' history will not be included in the master's history, but that's not the case now either. Because only the last amendment will be in the master's history (which is just as much a squash, except that that squash is the result if re-ammending over and over again, instead of subsequent commits being squashed afterwards). -- Krinkle [1] "branch-review model", as in, a model where a review is about a topic-branch, whether it contains 1 commit, or many. Or even a branch from a fork. In other words the pull-request model from GitHub. And yes, on GitHub one can also create a pull-request from one branch to another, within the same repository (e.g. mediawiki-core/feature-x -> mediawiki-core/master or mediawiki-core/feature-x-y-z -> mediawiki-core/feature-x) . [2] http://zachholman.com/talk/how-github-uses-github-to-build-github _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
