-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 17/06/14 12:36, Sean Dague wrote: > On 06/17/2014 07:23 AM, Daniel P. Berrange wrote: >> On Tue, Jun 17, 2014 at 11:04:17AM +0100, Matthew Booth wrote: >>> We all know that review can be a bottleneck for Nova >>> patches.Not only that, but a patch lingering in review, no >>> matter how trivial, will eventually accrue rebases which sap >>> gate resources, developer time, and will to live. >>> >>> It occurs to me that there are a significant class of patches >>> which simply don't require the attention of a core reviewer. >>> Some examples: >>> >>> * Indentation cleanup/comment fixes * Simple code motion * File >>> permission changes * Trivial fixes which are obviously correct >>> >>> The advantage of a core reviewer is that they have experience >>> of the whole code base, and have proven their ability to make >>> and judge core changes. However, some fixes don't require this >>> level of attention, as they are self-contained and obvious to >>> any reasonable programmer. >>> >>> Without knowing anything of the architecture of gerrit, I >>> propose something along the lines of a '+1 (trivial)' review >>> flag. If a review gained some small number of these, I suggest >>> 2 would be reasonable, it would be equivalent to a +2 from a >>> core reviewer. The ability to set this flag would be a >>> privilege. However, the bar to gaining this privilege would be >>> low, and preferably automatically set, e.g. 5 accepted patches. >>> It would be removed for abuse. >>> >>> Is this practical? Would it help? >> >> You are right that some types of fix are so straightforward that >> most reasonable programmers can validate them. At the same time >> though, this means that they also don't really consume >> significant review time from core reviewers. So having >> non-cores' approve trivial fixes wouldn't really reduce the >> burden on core devs. >> >> The main positive impact would probably be a faster turn around >> time on getting the patches approved because it is easy for the >> trivial fixes to drown in the noise. >> >> IME any non-trivial change to gerrit is just not going to happen >> in any reasonably useful timeframe though. Perhaps an >> alternative strategy would be to focus on identifying which the >> trivial fixes are. If there was an good way to get a list of all >> pending trivial fixes, then it would make it straightforward for >> cores to jump in and approve those simple patches as a priority, >> to avoid them languishing too long. >> >> If would be nice if gerrit had simple keyword tagging so any >> reviewer can tag an existing commit as "trivial", but that >> doesn't seem to exist as a concept yet. >> >> So an alternative perhaps submit trivial stuff using a well >> known topic eg >> >> # git review --topic trivial >> >> Then you can just query all changes in that topic to find easy >> stuff to approve. > > It could go in the commit message: > > TrivialFix > > Then could be queried with - > https://review.openstack.org/#/q/message:TrivialFix,n,z > > If a reviewer felt it wasn't a trivial fix, they could just edit > the commit message inline to drop it out.
+1. If possible I'd update the query to filter out anything with a -1. Where do we document these things? I'd be happy to propose a docs update. Matt - -- Matthew Booth Red Hat Engineering, Virtualisation Team Phone: +442070094448 (UK) GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlOgML0ACgkQNEHqGdM8NJCmpgCfbSy9bljyEqksjrJ7oRjE8LNH 8nUAoJBE5L+uAcQbew5ff/98eeqoRvW2 =+SOM -----END PGP SIGNATURE----- _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev