On Tue, Aug 9, 2011 at 1:29 PM, Monty Taylor <[email protected]> wrote:
> > > On 08/09/2011 01:28 PM, Jay Pipes wrote: > > On Tue, Aug 9, 2011 at 1:01 PM, Dan Prince <[email protected]> > 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" <[email protected]> > >> Sent: Monday, August 8, 2011 2:50pm > >> To: [email protected] > >> 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 : [email protected] > >> 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 : [email protected] > > Unsubscribe : https://launchpad.net/~openstack > > More help : https://help.launchpad.net/ListHelp > > > > _______________________________________________ > Mailing list: https://launchpad.net/~openstack > Post to : [email protected] > Unsubscribe : https://launchpad.net/~openstack > More help : https://help.launchpad.net/ListHelp >
_______________________________________________ Mailing list: https://launchpad.net/~openstack Post to : [email protected] Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
