Looks good except I think that sending paches to impdev is not a great
way to raise changes for discussion. An English description and
proposed function signatures (if non trivial functions are propsed) is
probably more useful ((and both patches and English is a bit much)
On Oct 20, 2008, at 12:07 AM, Notification of IMP commits <
> wrote:
Author:
Date: 2008-10-20 00:07:58 -0700 (Mon, 20 Oct 2008)
New Revision: 770
Modified:
trunk/doc/usage.xml
Log:
Document
- commit writes for kernel and modules
- modules outside of the IMP repository
- namespace macros
- code maturity
- finding and fixing bugs
Modified: trunk/doc/usage.xml
===================================================================
--- trunk/doc/usage.xml 2008-10-20 05:38:34 UTC (rev 769)
+++ trunk/doc/usage.xml 2008-10-20 07:07:58 UTC (rev 770)
@@ -152,11 +152,21 @@
There are also two "general purpose" modules: the <filename>misc</
filename>
module, which contains miscellaneous features which are
generally <emphasis>not</emphasis> used by other modules, and
-the <filename>core</filename> module, which contains general-
purpose features
-and upon which many other modules depend. The <filename>misc</
filename> module
-can be used for features that do not neatly fit into a specialized
module,
-such as code that implements proposed new methodologies.
+the <filename>core</filename> module, which contains features
expected to be
+used by almost everybody, and upon which many other modules depend.
+The <filename>misc</filename> module can be used for features that
do not
+neatly fit into a specialized module, such as code that implements
proposed
+new methodologies.
</para>
+
+<para>
+It is also possible to create your IMP modules outside of IMP
itself. This
+is ideal if you have different licensing requirements, your code is
very
+experimental, or you do not want to share your source code, for
example.
+Of course, if you go down this route, you must do your own book
keeping to
+make sure that the IMP kernel and any modules you depend on are
correctly built
<para>Exceptions are classes, so name as for classes, above. Do not
use
-exception specfications, but do document the possible exceptions
thrown by
+exception specifications, but do document the possible exceptions
thrown by
a function with the Doxygen <command>\exception</command> command.
</para>
@@ -297,6 +307,11 @@
Avoids namespace pollution, and removes any ambiguity.
</para>
+<para>Use the provided macros such as <code>IMP_BEGIN_NAMESPACE</
code> and
+<code>IMP_END_NAMESPACE</code> to begin and end the namespace for
the kernel
-<para>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
-<email></email> mailing list. This is mandatory
for
-changes that:</para>
+<sect2 id="choose-module">
+<title>Choosing a module for your code</title>
+
+<para>
+By far the easiest way to contribute to IMP is to create a module
for your
+specific problem domain. For example, restraints and optimizers
specific to
+ligands could go in a <filename>ligand</filename> module. Please
contact Ben
+to have a new module created. If you feel your code doesn't belong
in its
+own module, you can add it to the <filename>misc</filename> module
instead.
+If you feel your new functionality is of use to everybody and
should live in the
+<filename>core</filename> module, you should request this on the
IMP mailing
+list and have it approved by Ben, who has the final say on where
code should
+end up. This is partly to prevent the <filename>core</filename>
module from
+becoming too bloated with specialized code.
+</para>
+
+<para>
+Code should generally <emphasis>not</emphasis> be moved from one
+module to another, unless there is an exceptionally good reason to
do so. If
+for some reason you do want to move things between modules, ask Ben
to do it.
+</para>
+</sect2>
+
+<sect2 id="define-interfaces">
+<title>Defining your interfaces</title>
+
+<para>
+Before you sit down and write your new code or change existing
code, you
+should think about the interfaces. These are the classes and their
methods,
+without any implementation. It is recommended that you discuss these
+interfaces on the <email></email> mailing list
before you
+proceed with implementation, so that you do not duplicate work,
others can
+make valuable suggestions or point out problems, and people are
made aware of
+your contribution. If you want to add to or change the interfaces
in the
+kernel or the <filename>core</filename> module, this discussion is
+mandatory, since potentially everybody could be affected by your
changes. Any
+issues raised by users with changes to the kernel or
<filename>core</filename>
+must be resolved to their satisfaction before the changes are made.
For clarity,
+the following should be considered changes that warrant discussion:
+</para>
+
<itemizedlist>
<listitem><para>Change the API (e.g. remove or rename existing
methods or
classes, change the arguments taken by existing methods, change the
behavior
@@ -364,10 +415,25 @@
<listitem><para>Remove or modify existing test cases.</para></
listitem>
</itemizedlist>
-<para>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 <emphasis>required</emphasis> but is recommended.</
para>
+<para>
+Conversely, changes in internal implementation or documentation to
any module,
+or changes in the interfaces of your own modules or the
+<filename>misc</filename> module, do not need to be announced, and
neither does
+the addition of new methods to existing classes, although it may be
helpful
+to tell people about such changes.
+</para>
+</sect2>
+
+<sect2 id="patches">
+<title>Creating a patch</title>
+
+<para>
+Generally speaking, to add code to IMP, your changes should be made
to your
+checkout of the IMP repository, and then a patch generated by using
the
+<command>svn di</command> command.
+</para>
+
<para>Patches should contain a related set of changes. For example,
a patch
which adds a new method <methodname>foo</methodname>, a new testcase
for
<methodname>foo</methodname>, and some documentation for the
@@ -384,17 +450,97 @@
other effects is better than nothing.)</para>
<para><emphasis>Rationale:</emphasis>
-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. Test cases also help to
document the
+It is much easier for others to understand your patch's changes if
it is small
+and contains only relevant changes. Test cases also help to
document the
intent of code changes, as well as making sure that the new
additions are not
accidentally broken in future.
</para>
+</sect2>
+
+<sect2 id="commit-patch">
+<title>Committing a patch</title>
+
+<para>
+Once your patch is complete and you have run the testcases to make
sure you
+didn't break anything, and reviewed the patch to check your work,
it can be
+committed to the source control system, provided that any interface
changes
+have <link linkend="define-interfaces">been discussed</link> if
required.
+How this is done depends on whether you have commit rights for the
affected
+module(s). If you do, you can simply use <command>svn ci</command>.
If you do
+not, send the patch to the owner of the module for integration,
with a brief
+description of your changes for the source control logs:
+</para>
+
+<sect3 id="kernel-ci">
+<title>IMP kernel</title>
+<para>
+All changes to the IMP kernel, since they affect everybody, must be
approved
+by Ben. Send your patch to <email></email> for
integration.
+</para>
+</sect3>
+
+<sect3 id="core-misc-ci">
+<title>core and misc modules</title>
+<para>
+Anybody can commit to these modules after approval; ask Ben to get
your
+IMP is a work shared by many developers, and they would prefer not
to have
+spend time working around problems introduced by others elsewhere
in the code.
+Therefore, it is expected that any code committed to IMP should at
least
+compile. Any test cases you write for your code should also be
expected to
+pass, or at least if they do not, you should quickly fix things so
that they
+do. (As a rule of thumb, your changes should not break the nightly
builds
+for more than 3 consecutive days; if they do, your changes risk
being reverted.)
+</para>
+
+<para>
+It is fine to add code to IMP that is not yet fully complete or
ready for
+others to use (provided that it compiles and tests, as above).
However, any
+such classes or methods should be marked in their Doxygen comments as
+immature.
+</para>
+
+</sect2>
+
+<sect2 id="bugs">
+<title>Finding and fixing bugs in IMP</title>
+<para>
+If you find a bug in IMP, please report it on the
+<ulink url="https://salilab.org/imp/bugs/";>IMP bug tracker</ulink>.
This will
+ensure it does not get lost. The best way to report a bug is to
provide a
+short script file that demonstrates the problem.
+</para>
+
+<para>
+If you create a patch which fixes a bug, please cite the bug ID in
the commit
+log for that patch. The patch should also include a testcase which
triggers
+the bug, so that we can ensure your fix for the bug does not break
in future.
+In many cases, the demonstration script in the bug report can be
trivially