It seemed sensible to me to come up with a default IMP code review
policy. Please take a look over it and if you have any issues with it,
think it needs to be clarified some more, etc., follow up to the list:
To ensure code quality, additions to the IMP kernel should be reviewed
by the other developers. In general, this means posting a patch with
some explanation of the changes (e.g. suitable for the source control
logs) to the <> mailing list. This is mandatory for
changes that:
* Change the API (e.g. remove or rename existing methods or classes,
change the arguments taken by existing methods, change the behavior of
existing methods, change the return values or thrown exceptions of
existing methods).
* Remove or modify existing test cases.
For more minor changes, such as adding new tests, methods and/or classes
(which may change the ABI but not the API), documentation updates, etc.
review is not required but is recommended.
Patches should contain a related set of changes. For example, a patch
which adds a new method foo, a new testcase for foo, and some
documentation for the SpecialVector class, should be split into two
patches: one for the foo method and test, and the other for the
SpecialVector documentation.
New code in patches should also adhere to the coding style guidelines.
Rationale: Other developers may have external applications and their own
in-progress work which may be adversely affected by your changes. There
may also be disagreement about a change, so discussing it on the mailing
list first assures that a consensus is first reached, rather than other
developers later reverting the change. It is much easier to review a
patch if it is small and contains only relevant changes, and in addition
it can generally be applied to source control virtually unchanged if the
code is OK.
Ben
--
http://salilab.org/~ben/
"It is a capital mistake to theorize before one has data."
- Sir Arthur Conan Doyle