Open Stack

On Wed, Jun 18, 2014 at 11:04:15AM +0200, Thierry Carrez wrote:
> Russell Bryant wrote:
> > On 06/17/2014 08:20 AM, Daniel P. Berrange wrote:
> >> On Tue, Jun 17, 2014 at 01:12:45PM +0100, Matthew Booth wrote:
> >>> On 17/06/14 12:36, Sean Dague wrote:
> >>>> 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.
> >>
> >> Yes, that would be a workable idea.
> >>
> >>> +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.
> >>
> >> Lets see if any other nova cores dissent, but then can add it to these 2
> >> wiki pages
> >>
> >>   https://wiki.openstack.org/wiki/ReviewChecklist
> >>   https://wiki.openstack.org/wiki/GitCommitMessages#Including_external_references
> > 
> > Seems reasonable to me.
> > 
> > Of course, I just hope it doesn't put reviewers in a mode of only
> > looking for the trivial stuff and helping less with the big stuff.
> 
> As an aside, we don't really need two core reviewers to bless a trivial
> change: one could be considered sufficient. So a patch marked as trivial
> which has a number of +1s could be +2/APRVed directly by a core reviewer.
> 
> That would slightly reduce load on core reviewers, although I suspect
> most of the time is spent on complex patches, and trivial patches do not
> take that much time to process (or could even be seen as a nice break
> from more complex patch reviewing).

Agreed, I think this actually would help improve velocity in many cases,
provided there was sufficient common understanding of what consititutes a
trivial patch.

The other situation this may make sense is when a non-trivial change has
already been widely reviewed and approved then needs a last-minute minor
rebase to resolve a simple merge conflict (e.g due to all these trivial
patches suddenly landing really fast.. ;)

We discussed this in the Heat team a while back and agreed that having one
core re-review and approve the patch was sufficient, and basically that
folks could use their discretion in this situation.

Steve