On Sat, Jun 30, 2012 at 4:23 PM, Faidon Liambotis <fai...@wikimedia.org> wrote: > Well, in the ops puppet repo though, we very often +2 commits ourselves > and push them, instead of waiting for someone else to review/approve > them. You could argue that it's our workflow that it's wrong, but I just > think the needs for infrastructure-as-code might be different from the > needs code development has. > > It's like asking for pre-execution reviews of whatever you type in a > shell prompt and we can all agree that's just crazy :) In a perfect > world we'd stage every change in VMs where we'd do local puppet commits > without reviews; then push those tested changesets into the pre-commit > review system to get into production. But we're very far from that and > being perfectionists just hurts us more on our daily work. > > Having a proper post-commit review workflow would be much better than > hacking around the system and reviewing commits ourselves. It'd also > allow us to actually have post-commit reviews, something that rarely > happens right now. At least I'd do that, while currently it's a PITA to > do so. > Yes, ops essentially uses a post-commit workflow right now, and that makes sense for them. Gerrit doesn't support post-commit review very well so Barkeep might work better there. But other projects, such as MediaWiki core, do use real pre-commit (or pre-merge, rather) review.
> Barkeep claims to work with both post- and pre-commit workflows, > although the details elude me. > Per Subbu's message, you'd have to add support for pre-commit. Gerrit manages the git repositories themselves, implements fine-grained permission settings and handles gated branches both in terms of access control (no one can push directly to master, only certain people can approve things, etc.) and automatic merging. > The UI is much *much* nicer than Gerrit's. They also have a demo > website, which is a pleasure to work with IMHO. > Agreed. > They also claim to have useful, relevant and configurable e-mail > notifications too, in contrast to Gerrit's which are basically useless. > Maybe I'm too much of a relic to prefer reading commit diffs in my mail > client, rather than fancy web interfaces :) > I don't know what these "useful" e-mail notifications look like but I would love to find out. Gerrit's aren't totally useless but they're also not all that useful. > All in all, I like it very much but I don't have a broad knowledge of > how people use Gerrit right now and therefore I can't form an opinion on > whether it's suitable for us. > I think that depends on your definition of "us". For the puppet repo, a mix of pre-commit and post-commit (post-commit for immediate changes made by ops people, pre-commit for changes submitted by non-ops people (staff or volunteers) and for ops changes that can afford to wait for review) would probably be best. I think a number of other projects, such as the analytics repos or extensions that aren't deployed on the WMF cluster, might prefer this as well. But for MediaWiki core and deployed extensions I'm pretty sure we'll want to stick with pre-commit review. This makes it sound like using both Gerrit and Barkeep together might work, but I don't think that's necessarily a good idea. These are the problems I imagine we'd have: * people having to learn two different tools in general: more learning, possible confusion over what lives where and when to use what * review comments on one commit are spread across two systems, and you can't see the Gerrit comments in Barkeep or vice versa * ops staff works mostly with Barkeep, non-ops and volunteers work mostly with Gerrit --> ops staff more likely to neglect Gerrit review queue So I think it would be much better if we could have proper integration between the two, or maybe even implement pre-commit review in Barkeep. The issue ticket that Subbu found [1] has a comment suggesting how a pre-commit workflow might be implemented: * push to feature branches * run a script that polls Barkeep for approved commits in feature branches and merges them into master Problems with this approach are: * you need access controls to prevent people from pushing to master ** this can be done with something like gitolite, or even with Gerrit * Gerrit's immediate-merge-upon-approval feature is very nice ** this is easy to implement if Barkeep offers an event stream * fixing things to address review isn't handled nicely ** to fix something you'd either add a fixup commit to your branch or create a new branch with an amended commit ** in both cases the commits are separate, so the review comments are spread out and aren't linked like patchsets in Gerrit ** it's easy to accidentally approve&merge an outdated branch, or to forget all fixup commits ** (the fixup workflow is flawed in general; with the amend/new-branch workflow, every single commit in master is safe to deploy, with the fixup workflow there can be commits that are broken, don't pass the tests, or even have syntax errors) Per the points above I think Gerrit's backend and workflow are actually more well-thought-out than people give them credit for. Only if&when Barkeep is (or is hacked to be) able to handle these things well, would I seriously consider it as an alternative to Gerrit. Some ideas for how fixup handling might be done in a Barkeep-like system: * make people push fixup commits into the same feature branch but: ** allow viewing and reviewing/commenting/approving both the individual commits and the branch as a whole ** somehow integrate the individual and combined views so comments show up in both (Gerrit does this for patchsets) ** when merging a multi-commit branch, squash it into a single commit before merging *** this probably means a web interface for determining the commit summary of the squashed commit would be needed ** I don't know how rebasing would be handled here * alternatively, make people amend their commits and submit amended commits to myfeaturebranch/1, myfeaturebranch/2, etc. ** this is basically the Gerrit model except branch names are used to link commits rather than Change-Ids ** the same integration described above would be needed, but if desired it could be more Gerrit-like ** handling rebasing is not a problem ** handling dependent commits (if you amend a commit, other commits that depend on it need to be rebased) is just as problematic as in Gerrit, although this could be mostly alleviated by better UI support (offer automatic rebasing in the UI; if that conflicts, tell the user to rebase manually and give them the required commands for easy copy-pasting) My ideal system would probably be the amend-into-numbered-branch model but with better handling for series of dependent commits based on the fixup model. > At least there's some competition in the space and other people having > the same problems as (at least) I do, that's good :) > Yes, I'm really happy Barkeep was made (and brought to my attention), and hopefully it can drive innovation in the direction we need it to go in. Roan [1] https://github.com/ooyala/barkeep/issues/254 _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
