Add reviewing checklist to docs/contributions.

There is some overlap with the style guide. Where the style guide aims to be descriptive, this document is simply a list of questions for the reviewer to ask when when reading a PR.
On Sat, Aug 2, 2014 at 12:24 AM, fwereade <notifications@github.com> wrote:

> In doc/contributions/review-checklist.md:
>
> > +
> > +- Is all functionality covered?
> > +- Are all error cases covered?
> > +- Function names describe what is being tested and the expected result (i.e. success or failure).
> > +- Could similar tests be combined into a table based test?
> > +- Should a table based test be broken out in more obvious tests? (this sometimes happens when there is too much logic or too many conditionals in the actual test block of the table based test).
> > +- Is there repeated test boilerplate which should be extracted in functions?
> > +- Is the appropriate base test suite used? (overuse of JujuConnSuite is common).
> > +- Is the test using the API?
> > +- Are the tests isolated from the environment? Could the test mock access to the API?
> > +- Is the suite registered with Gocheck? (gc.Suite(...))
> > +- Is setup and teardown done correctly?
> > +- Is external functionality being called that should be mocked instead?
> > +- Each suite is set up directly before tests for that suite.
> > +- Correct use of c.Check vs c.Assert.
> > +- TMP files should be given a prefix (in case cleanup fails) e.g. test-juju..
>
> temp files should always be inside a c.MkDir() anyway -- so long as you do
> that, you can call them what you like.
>
Respectfully, no.

Our test process produces enormous amounts of temp files, so for my sanity,
please give them a predictable prefix so they can be nuked when my /tmp
grows into the gigabyte range.


> —
> Reply to this email directly or view it on GitHub
> <https://github.com/juju/juju/pull/441/files#r15698241>.
>