openstack team mailing list archive

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

On Wed, Jun 27, 2012 at 03:24:21PM +0200, Vincent Untz wrote:
> Hi,
> 
> As a recent contributor to OpenStack, but with experience in other
> projects, I think moving in the directions you document would be good.
> And as you wrote, it's common practice in many many projects, which is
> another argument for this :-)
> 
> However, one comment:
> 
> Le mercredi 27 juin 2012, à 11:52 +0100, Daniel P. Berrange a écrit :
> > It might be mentioned that Gerrit's handling of patch series is not entirely
> > perfect. This is a not a valid reason to avoid creating patch series.
> 
> It'd be really great if we could first improve Gerrit to handle the
> patch series workflow in a better way. Without such a change, pushing
> patch series to Gerrit is really no fun for anyone :/
> 
> I've no idea if this is currently being worked on (at least, I don't
> really se an issue reported in Gerrit's issue tracker). Maybe we should
> sit down and at least document how we'd like to improve this specific
> workflow?

Yep, no argument that Gerrit could do with some improvements, but having
submitted a number of non-trivial patch series to Nova, I don't think
current Gerrit UI is a complete blocker to adoption. It is not ideal,
but it isn't too painful if you're aware of what to look for. I think
the main problem is that since the patch dependancies are not obvious
in the UI, reviewers tend to miss the fact that they're reviewing a
patch that's part of a series.

I submitted one bug against Gerrit already to improve the way it
deals with bug resolution wrt patch series

  https://bugs.launchpad.net/openstack-ci/+bug/1018013

One think people might not be aware of is that if you create a patch
series on a branch and push that branch using 'git review', then the
branch name becomes the Gerrit topic which provides an easy way to
see the entire series. Alternatively the blueprint or bug IDs might
be chosen to form the topic. For example my most recent series:

https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bug/1003373,n,z


I think the main patch display UI though needs to make it much more
obvious that a particular patch is part of a series so that reviewers
know to review the work as a whole. The UI does display dependencies.
eg see the "depends on" and "needed by" links:

  https://review.openstack.org/#/c/8694/

but I'd suggest that UI be changed so that instead of showing only the
previous and next, it displays all patches in the series at once.

Gerrit could also be less stupid about repeatedly trying & failing to
merge a patch due to missing dependent patches. In fact I'll file a
bug about that flaw now too.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|