I got curious about how workflows were implemented and since the codebase is pretty small, I started digging around their code, but then found some relevant information here: https://github.com/ooyala/barkeep/issues/254 So, it appears that workflow is not part of barkeep itself since barkeep and git repos are decoupled -- this makes the default workflow post-commit, and pre-commit workflows are something that have to be scripted externally. Not sure what is involved there, but it is probably a matter of time before sample scripts / guides are up. Also, possibly, this might enable different commit-review workflows for different groups, if something like that might be useful. Subbu.

On Sat, Jun 30, 2012 at 4:23 PM, Faidon Liambotis <faidon(a)wikimedia.org> wrote: 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. 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. Agreed. 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. 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. 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

On Sat, Jun 30, 2012 at 11:53 PM, Antoine Musso <hashar+wmf(a)free.fr> wrote: Yeah, that's right. What I meant to say (and thought I had said in some form later in that message) was that the puppet repo has post-commit review for most changes by ops staff, and pre-commit review for everything else (non-ops staff, volunteers, and certain changes by ops staff in some cases). Roan

Some notes on the UI interface: (i) it doesn't allow to review the commit message (ii) the github-like comment and diff UI is nice, but the diffs aren't as informative: they don't seem to highlight extra whitespace and tabs as well as Gerrit. (iii) the interface showing the files is less informative: no list of files already checked, all changes in one page (wonderful for the commits editing 10+ files). (iv) it doesn't seem to support successive patch sets for one change, so maybe it's more post-commit centered than workflow-agnostic. So what should we do? Create another commit to fix the commit? Where will be the comments? On several pages? How will there be linked? (v) The work to adapt the software from pre-commit to post-commit. I don't understand how to fix a review. (vi) I don't understand how to test a change (yes it's a post-commit workflow, so just git pull, but in the case we transform Barkeep to a pre-commit...). Globally, I don't share the general enthusiasm and feeling about the UI ; I really think the Gerrit interface is more functional and more ergonomic, where Barkeep only gives a "2.0 fresh look". If it's only an interface issue, I would strongly advice to follow the rule "When it's not broken, don't fix it" and keep Gerrit instead to waste time and resources to hack Barkeep to have it suiting our pre-commit workflow need. On Mon, Jul 2, 2012 at 12:06 AM, Ryan Lane <rlane32(a)gmail.com> wrote: -- Sébastien Santoro aka Dereckson http://www.dereckson.be/

I'm actually very surprised to see such a high number for MediaWiki core. Why is this the case? When you say committer==reviewer, so do you mean the reviewer did a +2 and merged? Does this take into account committers that also just added comments? This doesn't surprise me at all. I have a *really* hard time getting my extensions reviewed and I work for the foundation. I have to beg for reviews, and even then it can take up to a couple weeks sometimes. It's problematic. That said, I don't necessarily think my extensions need pre-merge review. I'd really like Gerrit to support post-merge review in some fashion. Things that are going into production should be reviewed, preferably pre-merge. It should definitely be reviewed before deployment. Insecure code on a wikimedia.org domain has the ability to affect everything else too. Self-review is so prevalent in puppet because it's very difficult to do operations any other way. We occasionally do 50-100 deployments a day. Labs isn't a fully functional replica of everything in production yet, either, so we have no way to stage deployments, which means we can't wait on reviews. That said, pre-merge review is working exactly as intended. The fact that nearly 20% of the merges are reviewed shows exactly how well it's working. If you take a look at that more closely, I'm betting you'll see that those commits are almost completely outside of the operations staff. That's an amazingly huge win for a team that had 0 merged commits outside of the operations staff. This process simply wouldn't work without pre-merge review. - Ryan

That's not true anymore. It's been possible to fully test puppet manifests before committing for over a month, now. Nearly two weeks ago, we even got rid of the test branch in favor of pushing fully finished changes into the production branch. This assumes that this piece of infrastructure is in Labs, of course. - Ryan

On Mon, Jul 2, 2012 at 9:59 PM, Rob Lanphier <robla(a)wikimedia.org> wrote: I would agree with Ryan and say that it's not that we're not proud of this, it's that we have a different workflow. There's a lot of repetitive style work in our job (putting new servers in puppet and dhcp files, for example). These minor commits don't need any major review. Major changes can be tested in labs, usually have someone else check them out, and for many changes the worst breakage that happens is that puppet stops running(instead of a dead site). Leslie -- Leslie Carr Wikimedia Foundation AS 14907, 43821 http://as14907.peeringdb.com/

I have explored how to make a gerrit-substitute and it doesn't seem that hard. Initially, I planned to base on gitloite, but its workflow doesn't suit with gerrit. Basically, you would use a hook to rewrite the push references, and then run a script to register it in the db and UI (I did some simple tests). You just need some glue to make the UI perform git changes (merges) and a fancy display of the changes. Things like command filtering are tricky, but could be obtained from gitolite. The best of making a gerrit-similar solution is that it would be able to show gerrit history, thus fully replacing it (no need to hunt for old changesets in Special:Code/Gerrit/newtool). I expect a functonal gerrit clone could be made in a few weeks. (disclaimer: I don't have such weeks available now, while I may advance it on the future, don't expect me to come with an alternative tomorrow, OTOH if someone wants to give it a go...)