<div class="gmail_quote">On Tue, Aug 9, 2011 at 1:29 PM, Monty Taylor <span dir="ltr"><<a href="mailto:mordred@inaugust.com">mordred@inaugust.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
<br>
On 08/09/2011 01:28 PM, Jay Pipes wrote:<br>
> On Tue, Aug 9, 2011 at 1:01 PM, Dan Prince <<a href="mailto:dan.prince@rackspace.com">dan.prince@rackspace.com</a>> wrote:<br>
>> Couple of comments:<br>
>><br>
>> 1) While gerrit is integrated w/ Launchpad (and can close tickets) Launchpad<br>
>> is not integrated with Gerrit. Things like referencing a branch from within<br>
>> a ticket or blueprint aren't going to work as well as they used to right?<br>
<br>
This is correct (although it's certainly something that we could look in<br>
to over time - launchpad is open source and their stated goal is<br>
integration with upstream projects - so perhaps expanding branch links<br>
to include external branches is something possible - but certainly not<br>
in the next six months)<br></blockquote><div><br>My biggest question is why are we spending so much time and effort integrating Launchpad into this new process? It seems like we're still depending on it quite a bit, when I thought the original goal was to move away from it.<br>
<br>Our process requirements were/are based on the LP workflow -- all we're doing here is trying to reinvent the wheel by using Gerret. I propose that if we want to stick with the exact same process (and not willing to figure out a new way to handle the GitHub reviews) that we just stick with LP. We're not gaining much by just switching from bzr to git, IMHO.<br>
 </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
> Actually, this is being worked on:<br>
> <a href="https://bugs.launchpad.net/openstack-ci/+bug/823173" target="_blank">https://bugs.launchpad.net/openstack-ci/+bug/823173</a><br>
<br>
Well, and there's that. :)<br>
<br>
>> 2) I'd like to see a unified diff containing all the files on the Gerrit<br>
>> review pages. Is there a way to do this or am I missing something?<br>
><br>
> Also being worked on, by the Nokia team and should be in the next<br>
> release of Gerrit:<br>
> I think this is the feature issue:<br>
> <a href="http://code.google.com/p/gerrit/issues/detail?id=106" target="_blank">http://code.google.com/p/gerrit/issues/detail?id=106</a><br>
><br>
> That being said, after using the "next changed source file" links,<br>
> I've actually changed my mind about using a single diff for larger<br>
> commits with lots of files changed. Sometimes it's actually nice to<br>
> scroll through them. But, having the option to have a single page diff<br>
> would be useful at times, too...<br>
<br>
I, for one, definitely want the single-page review- but also agree with<br>
Jay, the next-file isn't as bad as I thought originally. (also, gerrit<br>
is very ajax heavy- so most of those buttons/links are doing internal<br>
state changes and not reloads - so the next page links should be snappy)<br>
<br>
>> 3) The branch/refspec names Gerrit uses are not very user friendly. In<br>
>> Launchpad we typically had people naming their branches w/ either a<br>
>> feature/fix name or the ticket number. So in Launchpad my branch would be<br>
>> called something like 'lp:~dan-prince/fix_ec2_metadata' or whatever. In<br>
>> Gerrit the branch names up for review are rather cryptic<br>
>> 'refs/changes/76/176/1' which means that when trying to track and Gate<br>
>> branches before they hit trunk we are going to manually have to do an extra<br>
>> bit of detective work to make sense of which tickets and features a<br>
>> particular refspec corresponds to. Some extra tooling might might this<br>
>> easier but I really dislike that we have no control over the branch names<br>
>> that are up for review.<br>
><br>
> I will let Jim and Monty talk about this.<br>
<br>
Well- there are some interrelated things here. I'm going to try to talk<br>
about them sensibly - but may ramble - excuse me for that please. :)<br>
<br>
One is the branch/refspec names. In launchpad, there are named branches<br>
someone pushes (like dan-prince/fix_ec2_metadata) - then there are<br>
possible bug tags (commit --fixes=lp:213413) and then there are merge<br>
prop descriptions and commit messages. When we rolled launchpad out<br>
originally, there was a decent sized complaint that a merge prop would<br>
have an additional comment.<br>
<br>
In gerrit - there are no extra descriptions or commit messages because<br>
it uses the commit message from git. Additionally - it uses the first<br>
line of the commit message as a commit "subject" (which is a git common<br>
practice) so that the list of pending changes actually has a descriptive<br>
identifier if you wrote a good commit message.<br>
<br>
So from a "look at the list of things to review" standpoint, I actually<br>
feel like I have a much _clearer_ understanding of what each thing is<br>
before I review it than I did just looking a list of branch names on<br>
launchpad.<br>
<br>
>From the perspective of associating proposed changes with<br>
bugs/blueprints - we currently have a gerrit hook which will do regex<br>
matching on commit messages to look for bug numbers and link things to<br>
the launchpad bug. So as long as you say something like "Bug 235153:<br>
Fixed ec2 problem" - this should work both for automatic scriptable<br>
linking and user semantic understanding.<br>
<br>
The key here is that the branch name or refspec isn't the list you<br>
should be looking for identifying info - it's the commit subject (which<br>
also means less duplication of effort on a dev's side) If something is<br>
wanting to process this that is not jenkins (such as smokestack) ... the<br>
commit message is in the json object returned by the gerrit query<br>
command - so it should be fairly simple to do the same or similar<br>
parsing to show a description of the branch being tested.<br>
<br>
Is that helpful?<br>
<br>
>> Lastly I kind of feel like we've bee dupped. When we talked about Git code<br>
>> hosting on GitHub at the conference some of the major points were improved<br>
>> code review (on GitHub) and performance improvements for checkouts, etc.<br>
>> While we may have taken a step forward on the performance front IMHO we've<br>
>> taken a major step backwards as far as the review and tracking process goes.<br>
><br>
> I explained the deficiencies in GitHub's pull request system for<br>
> integration with an automated patch queue manager and gated trunk.<br>
> Could you point to a review you've done in Gerrit so far that we can<br>
> use as a starting point for discussion on what improvements can be<br>
> made relative to the "GitHub process"? I couldn't find any reviews<br>
> that you've done in Gerrit.<br>
<br>
I certainly don't want anyone to feel duped here. We did start down the<br>
road quite earnestly of using github, and could not meet all of the<br>
requirements that we have currently as a project. At that point, we<br>
moved on to attempt to solve the underlying problem that people had<br>
expressed in the best way possible - which I'm sure you can understand<br>
given the large number of people with conflicting interests was ... fun.<br>
<br>
In looking at things - it seemed that, to be quite honest, the number<br>
one single change that would make the maximum number of people happy was<br>
switching from bzr to git, and there was nothing about git that would<br>
not meet the needs of the project. (it is, of course, an excellent tool)<br>
<br>
All in all, I'm not saying that all of the design choices are perfect,<br>
and there are certainly things to work on - but I _do_ think that we're<br>
in an excellent position now to actually effect the changes that we<br>
need. (which will make it more effective to sit down at design summits<br>
and discuss needs - since we should be able to actually implement them)<br>
<br>
Thanks!<br>
Monty<br>
<br>
>> -----Original Message-----<br>
>> From: "Jay Pipes" <<a href="mailto:jaypipes@gmail.com">jaypipes@gmail.com</a>><br>
>> Sent: Monday, August 8, 2011 2:50pm<br>
>> To: <a href="mailto:openstack@lists.launchpad.net">openstack@lists.launchpad.net</a><br>
>> Subject: [Openstack] Status of Git/Gerrit Code Hosting/Review<br>
>><br>
>> Hello all,<br>
>><br>
>> tl;dr<br>
>> =======<br>
>><br>
>> Contributors have been giving Monty Taylor and Jim Blair feedback on<br>
>> the Gerrit code review system over the last few weeks. Both the<br>
>> Keystone and Glance projects have now migrated to using Git as their<br>
>> source control system and Gerrit for code review and integration into<br>
>> the Jenkins continuous integration system.<br>
>><br>
>> Tomorrow, the Project Policy Board (PPB) will be voting on two things:<br>
>><br>
>> 1) Should OS projects<br>
>> a) have a vetted set of options for hosting and review, or<br>
>> b) be required to use a single toolset for review and hosting<br>
>> 2) Shall Gerrit+Git be included in the set of vetted options or be the<br>
>> single option (dependent on the vote result for 1) above)<br>
>><br>
>> Feedback on #2 is most welcome. Please feel free to respond to this<br>
>> email, catch us on IRC or email me directly.<br>
>><br>
>> Links:<br>
>><br>
>> Working with Gerrit: <a href="http://wiki.openstack.org/GerritWorkflow" target="_blank">http://wiki.openstack.org/GerritWorkflow</a><br>
>> Code Review in Gerrit: <a href="http://review.openstack.org" target="_blank">http://review.openstack.org</a><br>
>><br>
>> Details<br>
>> =======<br>
>><br>
>> Over the last few weeks, Monty Taylor and Jim Blair have been working<br>
>> with a number of OpenStack contributors to gather feedback on a<br>
>> Git-based development workflow, toolset, and review process.<br>
>><br>
>> First, Monty and Jim investigated whether GitHub's pull request system<br>
>> would be sufficient to enforce existing code review and approval<br>
>> policies. It was determined that GitHub's pull request system was not<br>
>> sufficient. The main reason why the pull request system failed to meet<br>
>> needs is that there is no overall way to track the current state of a<br>
>> given pull request. While this is fine for the simple case (merge<br>
>> request is accepted and merged) it starts to fall over with some of<br>
>> the more complex back and forths that we wind up having in many<br>
>> OpenStack projects. Additionally, this assessment was predicated on<br>
>> the current design of a gated trunk with an automated patch queue<br>
>> manager, and a system where a developer is not required to spend time<br>
>> landing a patch (other than potential needs for rebases or changes due<br>
>> to code review).<br>
>><br>
>> Monty and Jim then decided to set up a Gerrit server for code review<br>
>> and CI integration at <a href="http://review.openstack.org" target="_blank">http://review.openstack.org</a>. Gerrit is a tool<br>
>> developed by Google to address some of the functionality the Android<br>
>> Open Source team needed around automated patch queue management and<br>
>> code reviews.<br>
>><br>
>> The first project that moved from Launchpad to Gerrit/Git was the<br>
>> openstack-ci project. This is the glue code and scripts that support<br>
>> the continuous integration environment running on<br>
>> <a href="http://jenkins.openstack.org" target="_blank">http://jenkins.openstack.org</a>.<br>
>><br>
>> After gaining some experience with Gerrit through the migration of<br>
>> this project from Launchpad, the next OpenStack subproject to move to<br>
>> the Gerrit platform was the Keystone incubated project. Keystone was<br>
>> already using git for source control and was on GitHub, using GitHub's<br>
>> Issues for its pull requests and bug tracking. However, the Keystone<br>
>> source code was not gated by a non-human patch queue management<br>
>> system; a Keystone developer would manually merge proposed branches<br>
>> into the master Keystone source tree, and code reviews were not passed<br>
>> through any automated tests on <a href="http://jenkins.openstack.org" target="_blank">http://jenkins.openstack.org</a>. Monty and<br>
>> Jim worked with Dolph, Yogi, Ziad, and other Keystone developers to<br>
>> get their code reviews done via Gerrit and get their unit and<br>
>> functional tests running on each commit through Jenkins. There were a<br>
>> few hiccups along the way, but the hiccups served as valuable lessons<br>
>> and were documented in the workflow wiki page<br>
>> <a href="http://wiki.openstack.org/GerritWorkflow" target="_blank">http://wiki.openstack.org/GerritWorkflow</a>.<br>
>><br>
>> Last Thursday morning, the Glance project was migrated from Bazaar and<br>
>> Launchpad code hosting to use Git and Gerrit. The migration went<br>
>> pretty smoothly, and a number of Glance developers have already been<br>
>> proposing, reviewing, and approving code via Gerrit. Launchpad is used<br>
>> for all bug tracking and blueprint management, still, but the code at<br>
>> <a href="http://code.launchpad.net/glance" target="_blank">http://code.launchpad.net/glance</a> is merely a read-only mirror of the<br>
>> git repository.<br>
>><br>
>> Outstanding Issues/Questions<br>
>> =====================<br>
>><br>
>> 1) John Dickinson and Chuck Thier raised the question that if Gerrit<br>
>> is going to be the (or one of the) proposed code review and patch<br>
>> management system, that hosting Git repositories on GitHub might be<br>
>> confusing for GitHub users, since most would expect to use pull<br>
>> requests to merge their own code back into the project's master repo.<br>
>> This is a valid concern and Monty and Jim are investigating<br>
>> establishing a GitWeb or Gitorious server on <a href="http://git.openstack.org" target="_blank">http://git.openstack.org</a><br>
>> that would serve as the canonical Git repo locations for OpenStack<br>
>> projects instead of GitHub. This would be similar to how<br>
>> <a href="http://git.kernel.org" target="_blank">http://git.kernel.org</a> works<br>
>><br>
>> 2) Only code hosting has been moved to Git/Gerrit. There are currently<br>
>> no plans to discuss moving bug tracking for existing OpenStack core<br>
>> projects to GitHub Issues. Gerrit is fully integrated with Launchpad<br>
>> bug tracking. This means that Gerrit *will close* (mark Fix Committed)<br>
>> Launchpad bugs if you include bug text in your commit message.<br>
>><br>
>> 3) The user interface for Gerrit is UGLY. I don't think anyone would<br>
>> disagree with that. :) That said, Gerrit's UI can be modified via CSS<br>
>> and templates without having to keep a separate fork of Gerrit. If you<br>
>> are interested in helping Monty and Jim make the Gerrit UI prettier<br>
>> and saving reviewers eyeballs, please do contact me.<br>
>><br>
>> Cheers,<br>
>> -jay<br>
>><br>
>> _______________________________________________<br>
>> Mailing list: <a href="https://launchpad.net/%7Eopenstack" target="_blank">https://launchpad.net/~openstack</a><br>
>> Post to : <a href="mailto:openstack@lists.launchpad.net">openstack@lists.launchpad.net</a><br>
>> Unsubscribe : <a href="https://launchpad.net/%7Eopenstack" target="_blank">https://launchpad.net/~openstack</a><br>
>> More help : <a href="https://help.launchpad.net/ListHelp" target="_blank">https://help.launchpad.net/ListHelp</a><br>
>> This email may include confidential information. If you received it in<br>
>> error, please delete it.<br>
>><br>
>><br>
><br>
> _______________________________________________<br>
> Mailing list: <a href="https://launchpad.net/%7Eopenstack" target="_blank">https://launchpad.net/~openstack</a><br>
> Post to     : <a href="mailto:openstack@lists.launchpad.net">openstack@lists.launchpad.net</a><br>
> Unsubscribe : <a href="https://launchpad.net/%7Eopenstack" target="_blank">https://launchpad.net/~openstack</a><br>
> More help   : <a href="https://help.launchpad.net/ListHelp" target="_blank">https://help.launchpad.net/ListHelp</a><br>
><br>
<br>
_______________________________________________<br>
Mailing list: <a href="https://launchpad.net/%7Eopenstack" target="_blank">https://launchpad.net/~openstack</a><br>
Post to     : <a href="mailto:openstack@lists.launchpad.net">openstack@lists.launchpad.net</a><br>
Unsubscribe : <a href="https://launchpad.net/%7Eopenstack" target="_blank">https://launchpad.net/~openstack</a><br>
More help   : <a href="https://help.launchpad.net/ListHelp" target="_blank">https://help.launchpad.net/ListHelp</a><br>
</blockquote></div><br>