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 :| _______________________________________________ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
