[controller-dev] Request Review: Formalize Committer Checklist for the Controller Project
davery at Brocade.com
Mon Jul 21 17:58:20 UTC 2014
Hi All -
A few of us wanted to clarify and enhance the requirements that committers use to push patches. At minimum we want to document the requirements for commits so it is clear to everyone committing to the controller (and to the reviewers) what it takes to get a change merged. We also want to propose some perhaps additional requirements that we want committers to look out for. So, with that said here are some ideas that multiple individuals have suggested that we ask the Controller committers to uphold:
* What ever expectations we arrive at should be advertised on the controllers project page. Information should probably include:
* Commit message format
* Short slug plus body?
* Required change-id?
* When to expect a patch will be reviewed or merged
* What to do if it’s not?
* In general as a committer your job is to (within reason) say no
* Merge Schedule:
* Bug fixes: Can be delivered anytime
* Low Risk Features: Can be merged Monday, Tuesday, Wednesday, Thursday
* High Risk Features: Can be merged Monday, Tuesday
* Is the code readable and legible?
* Is there valid Javadoc that will show up to help developers/users understand how to use the class/interface?
* This should be required for *every* public function and encouraged for all things.
* Are there enough comments? do they make sense?
* Is there logging? Does the logging make sense?
* For example: are there info / warning / error messages that would be printed often that produce a lot of content.
* Is there a license/copyright statement all new source files?
* Is this the right location in source code for this functionality?
* As an exercise, what is the second best place you can think of and does it fit better there?
* Who are they key stakeholders? Has anyone talked to them? If not, add them as reviewers.
* Is there enough diversity of reviewers?
* Ideally, people from more than one company.
* Are there adequate tests for the new functionality?
* If it's a bugfix, is there a test to prevent future regression?
* If it's a feature, are there tests to cover it?
We would appreciate feedback from everyone (including committers) on the controller project about whether we want to hold ourselves to these standards or a subset thereof etc. If folks have any other suggest please do share.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the controller-dev