At a minimum, a patch should:
- pass the entire test suite reliably
- pass 'make pyflakes'
- include documentation (in master/docs) for any new or changed functionality
- be licensed under the GPLv2 (this is assumed for any contribution; see LicensingYourContribution)
- be compatible with Python-2.5
In general, all new functionality should have tests. However, at the moment some parts of Buildbot have very few tests and are difficult to write tests for, so this is not a hard-and-fast rule.
Buildbot has had an inconsistent history with respect to docstrings. We now follow a policy of documenting something once. Thus, if a class is intended for use by users (e.g., a new step), it should be documented in the Texinfo documentation (master/docs) and not in a docstring. Conversely, if the changed code is part of an internal Buildbot interface, then it should have a docstring.
- overly long try blocks with a blanket except. These can tend to hide mistakes made when modifying the code.
- hard-coded values that users might want to adjust
- difficult-to-follow path of execution. Use nested functions for long deferred chains, rather than methods like _myMethod1, _myMethod2, etc.