[openstack-dev] Criteria for giving a -1 in a review

Matthew Booth mbooth at redhat.com
Thu Aug 21 16:05:04 UTC 2014


"I would prefer that you didn't merge this."

i.e. The project is better off without it.

This seems to mean different things to different people. There's a list
here which contains some criteria for new commits:

https://wiki.openstack.org/wiki/ReviewChecklist.

There's also a treatise on git commit messages and the structure of a
commit here:

https://wiki.openstack.org/wiki/GitCommitMessages

However, these don't really cover the general case of what a -1 means.
Here's my brain dump:

* It contains bugs
* It is likely to confuse future developers/maintainers
* It is likely to lead to bugs
* It is inconsistent with other solutions to similar problems
* It adds complexity which is not matched by its benefits
* It isn't flexible enough for future work landing RSN
* It combines multiple changes in a single commit

Any more? I'd be happy to update the above wiki page with any consensus.
It would be useful if any generally accepted criteria were readily
referenceable.

I also think it's worth explicitly documenting a few things we
might/should mention in a review, but which aren't a reason that the
project would be better off without it:

* Stylistic issues which are not covered by HACKING

By stylistic, I mean changes which have no functional impact on the code
whatsoever. If a purely stylistic issue is sufficiently important to
reject code which doesn't adhere to it, it is important enough to add to
HACKING.

* I can think of a better way of doing this

There may be a better solution, but there is already an existing
solution. We should only be rejecting work that has already been done if
it would detract from the project for one of the reasons above. We can
always improve it further later if we find the developer time.

* It isn't flexible enough for any conceivable future feature

Lets avoid premature generalisation. We can always generalise as part of
landing the future feature.

Any more of these?

Thanks,

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



More information about the OpenStack-dev mailing list