Open Stack

On Tue, Aug 9, 2011 at 1:29 PM, Monty Taylor <mordred at inaugust.com> wrote:

>
>
> On 08/09/2011 01:28 PM, Jay Pipes wrote:
> > On Tue, Aug 9, 2011 at 1:01 PM, Dan Prince <dan.prince at rackspace.com>
> wrote:
> >> Couple of comments:
> >>
> >> 1) While gerrit is integrated w/ Launchpad (and can close tickets)
> Launchpad
> >> is not integrated with Gerrit. Things like referencing a branch from
> within
> >> a ticket or blueprint aren't going to work as well as they used to
> right?
>
> This is correct (although it's certainly something that we could look in
> to over time - launchpad is open source and their stated goal is
> integration with upstream projects - so perhaps expanding branch links
> to include external branches is something possible - but certainly not
> in the next six months)
>

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.

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.


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