openstack team mailing list archive

[Monty Taylor <mordred@xxxxxxxxxxxx>]

On 08/09/2011 02:57 PM, Christopher MacGown wrote:
>> 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.
> 
> Given that you feel that git commit messages are sufficient for having
> a clear understanding of what each thing is before reviewing, I'm
> curious why comments within the github pull-request are not sufficient
> for an automated, clear gated queue manager. If it's a matter of
> preventing erroneous merges by *-core, that's solved by only allowing
> the merge-bot write access to the repository. Since every review on
> Launchpad tends to be accompanied by comments anyway, I'm not sure I
> agree that we lose anything by using the existing integrated service
> provided by github.
> 
> Arbitrary statuses can and should be handled by the automated
> merge-bot itself, if they're actually necessary (and I'm not convinced
> they are).

The statuses are necessary to understand workflow in the complex case.
In the simple case (review comes in, is accepted, is merged) all is just
fine and only using comments would be perfectly acceptable. In the more
complex case (such as often happens in nova, given the size of the dev
community, the amount of types of testing performed and the velocity of
changes) you have workflows that wind up having sequences such as:

- submit request
- review ack
- review ack
- branch is tested and failed, please fix
- branch fixed
- review ack
- branch is now out of date, please rebase
- branch fixed
- review ack
- branch is testsd and is merged

To expect a dev to read through all of the comments on each request to
see what state a request is in so that they know what might be expected
of them at this point (or, as a reviewer, to know if further action on
your part is needed) isn't particularly scalable. The _bot_ may be able
to easily understand what the current status of a request is - but the
dev, the pool of reviewers and people like the release manager would
really have no idea of the overall status of things or what they need to
do next.

Alternately - if you look at

https://review.openstack.org/#q,status:open,n,z

You quickly get an overview of:
 a) the purpose of each change
 b) what state it is in
And from that can derive an clear understanding of what steps you might
be expected to take next on any of these items.

Monty