From drussel@gmail.com Mon Oct 20 00:24:44 2008 From: Daniel Russel To: imp-dev@salilab.org Subject: Re: [IMP-dev] [IMP-commits] r770 - trunk/doc Date: Mon, 20 Oct 2008 00:24:35 -0700 Message-ID: <3109EDF0-9C37-42AD-BD8C-267E98B8E666@gmail.com> In-Reply-To: <200810200707.m9K77wxV025293@cowbell.compbio.ucsf.edu> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8366114968151657019==" --===============8366114968151657019== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Looks good except I think that sending paches to impdev is not a great =20 way to raise changes for discussion. An English description and =20 proposed function signatures (if non trivial functions are propsed) is =20 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: ben(a)SALILAB.ORG > 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 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- 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 misc filename> > module, which contains miscellaneous features which are > generally not used by other modules, and > -the core module, which contains general-=20 > purpose features > -and upon which many other modules depend. The misc filename> module > -can be used for features that do not neatly fit into a specialized =20 > module, > -such as code that implements proposed new methodologies. > +the core module, which contains features =20 > expected to be > +used by almost everybody, and upon which many other modules depend. > +The misc module can be used for features that =20 > do not > +neatly fit into a specialized module, such as code that implements =20 > proposed > +new methodologies. > > + > + > +It is also possible to create your IMP modules outside of IMP =20 > itself. This > +is ideal if you have different licensing requirements, your code is =20 > very > +experimental, or you do not want to share your source code, for =20 > example. > +Of course, if you go down this route, you must do your own book =20 > keeping to > +make sure that the IMP kernel and any modules you depend on are =20 > correctly built > +before your own. > + > > > > @@ -233,7 +243,7 @@ > > Exceptions > Exceptions are classes, so name as for classes, above. Do not =20 > use > -exception specfications, but do document the possible exceptions =20 > thrown by > +exception specifications, but do document the possible exceptions =20 > thrown by > a function with the Doxygen \exception command. > > > @@ -297,6 +307,11 @@ > Avoids namespace pollution, and removes any ambiguity. > > > +Use the provided macros such as IMP_BEGIN_NAMESPACE code> and > +IMP_END_NAMESPACE to begin and end the namespace for =20 > the kernel > +or a module. > + > + > > > > @@ -347,14 +362,50 @@ > > > > - > -Code review > + > +Adding code to IMP > > -To ensure code quality, additions to the IMP kernel should be =20 > reviewed > -by the other developers. In general, this means posting a patch =20 > with some > -explanation of the changes (e.g. suitable for the source control =20 > logs) to the > -imp-dev(a)salilab.org mailing list. This is mandatory =20 > for > -changes that: > + > +Choosing a module for your code > + > + > +By far the easiest way to contribute to IMP is to create a module =20 > for your > +specific problem domain. For example, restraints and optimizers =20 > specific to > +ligands could go in a ligand module. Please =20 > contact Ben > +to have a new module created. If you feel your code doesn't belong =20 > in its > +own module, you can add it to the misc module =20 > instead. > +If you feel your new functionality is of use to everybody and =20 > should live in the > +core module, you should request this on the =20 > IMP mailing > +list and have it approved by Ben, who has the final say on where =20 > code should > +end up. This is partly to prevent the core =20 > module from > +becoming too bloated with specialized code. > + > + > + > +Code should generally not be moved from one > +module to another, unless there is an exceptionally good reason to =20 > do so. If > +for some reason you do want to move things between modules, ask Ben =20 > to do it. > + > + > + > + > +Defining your interfaces > + > + > +Before you sit down and write your new code or change existing =20 > code, you > +should think about the interfaces. These are the classes and their =20 > methods, > +without any implementation. It is recommended that you discuss these > +interfaces on the imp-dev(a)salilab.org mailing list =20 > before you > +proceed with implementation, so that you do not duplicate work, =20 > others can > +make valuable suggestions or point out problems, and people are =20 > made aware of > +your contribution. If you want to add to or change the interfaces =20 > in the > +kernel or the core module, this discussion is > +mandatory, since potentially everybody could be affected by your =20 > changes. Any > +issues raised by users with changes to the kernel or =20 > core > +must be resolved to their satisfaction before the changes are made. =20 > For clarity, > +the following should be considered changes that warrant discussion: > + > + > > Change the API (e.g. remove or rename existing =20 > methods or > classes, change the arguments taken by existing methods, change the =20 > behavior > @@ -364,10 +415,25 @@ > Remove or modify existing test cases. listitem> > > > -For more minor changes, such as adding new tests, methods and/=20 > or classes > -(which may change the ABI but not the API), documentation updates, =20 > etc. > -review is not required but is recommended. para> > + > +Conversely, changes in internal implementation or documentation to =20 > any module, > +or changes in the interfaces of your own modules or the > +misc module, do not need to be announced, and =20 > neither does > +the addition of new methods to existing classes, although it may be =20 > helpful > +to tell people about such changes. > + > > + > + > + > +Creating a patch > + > + > +Generally speaking, to add code to IMP, your changes should be made =20 > to your > +checkout of the IMP repository, and then a patch generated by using =20 > the > +svn di command. > + > + > Patches should contain a related set of changes. For example, =20 > a patch > which adds a new method foo, a new testcase =20 > for > foo, and some documentation for the > @@ -384,17 +450,97 @@ > other effects is better than nothing.) > > Rationale: > -Other developers may have external applications and their own in-=20 > progress work > -which may be adversely affected by your changes. There may also be =20 > disagreement > -about a change, so discussing it on the mailing list first assures =20 > that a > -consensus is first reached, rather than other developers later =20 > reverting the > -change. It is much easier to review a patch if it is small and =20 > contains only > -relevant changes, and in addition it can generally be applied to =20 > source control > -virtually unchanged if the code is OK. Test cases also help to =20 > document the > +It is much easier for others to understand your patch's changes if =20 > it is small > +and contains only relevant changes. Test cases also help to =20 > document the > intent of code changes, as well as making sure that the new =20 > additions are not > accidentally broken in future. > > > + > + > + > +Committing a patch > + > + > +Once your patch is complete and you have run the testcases to make =20 > sure you > +didn't break anything, and reviewed the patch to check your work, =20 > it can be > +committed to the source control system, provided that any interface =20 > changes > +have been discussed if =20 > required. > +How this is done depends on whether you have commit rights for the =20 > affected > +module(s). If you do, you can simply use svn ci. =20 > If you do > +not, send the patch to the owner of the module for integration, =20 > with a brief > +description of your changes for the source control logs: > + > + > + > +IMP kernel > + > +All changes to the IMP kernel, since they affect everybody, must be =20 > approved > +by Ben. Send your patch to imp-dev(a)salilab.org for =20 > integration. > + > + > + > + > +core and misc modules > + > +Anybody can commit to these modules after approval; ask Ben to get =20 > your > +account activated. > + > + > + > + > +Other modules > + > +Domain-specific modules all have a responsible owner, and it is up =20 > to them > +whether they want to allow others commit rights. Email them and ask =20 > for > +commit rights (a sysadmin can add these), or send them your patch for > +integration. If in doubt, post it on the imp-=20 > dev(a)salilab.org > +mailing list. > + > + > + > + > + > + > +Code maturity > + > +IMP is a work shared by many developers, and they would prefer not =20 > to have > +spend time working around problems introduced by others elsewhere =20 > in the code. > +Therefore, it is expected that any code committed to IMP should at =20 > least > +compile. Any test cases you write for your code should also be =20 > expected to > +pass, or at least if they do not, you should quickly fix things so =20 > that they > +do. (As a rule of thumb, your changes should not break the nightly =20 > builds > +for more than 3 consecutive days; if they do, your changes risk =20 > being reverted.) > + > + > + > +It is fine to add code to IMP that is not yet fully complete or =20 > ready for > +others to use (provided that it compiles and tests, as above). =20 > However, any > +such classes or methods should be marked in their Doxygen comments as > +immature. > + > + > + > + > + > +Finding and fixing bugs in IMP > + > +If you find a bug in IMP, please report it on the > +IMP bug tracker. =20 > This will > +ensure it does not get lost. The best way to report a bug is to =20 > provide a > +short script file that demonstrates the problem. > + > + > + > +If you create a patch which fixes a bug, please cite the bug ID in =20 > the commit > +log for that patch. The patch should also include a testcase which =20 > triggers > +the bug, so that we can ensure your fix for the bug does not break =20 > in future. > +In many cases, the demonstration script in the bug report can be =20 > trivially > +adapted into a test case. > + > + > + > + > > > > > _______________________________________________ > IMP-commits mailing list > IMP-commits(a)salilab.org > https://salilab.org/mailman/listinfo/imp-commits --===============8366114968151657019==-- From ben@salilab.org Mon Oct 20 00:28:33 2008 From: Ben Webb To: imp-dev@salilab.org Subject: Re: [IMP-dev] [IMP-commits] r770 - trunk/doc Date: Mon, 20 Oct 2008 00:28:33 -0700 Message-ID: <48FC3321.3000903@salilab.org> In-Reply-To: <3109EDF0-9C37-42AD-BD8C-267E98B8E666@gmail.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5977801081119290182==" --===============5977801081119290182== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Daniel Russel wrote: > 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) Exactly - that's what I say, isn't it? Maybe the text is misleading in some way? I just say "discuss these interfaces on the imp-dev mailing list" not the implementations. Or perhaps you're referring to a different part of the text? Ben -- ben(a)salilab.org http://salilab.org/~ben/ "It is a capital mistake to theorize before one has data." - Sir Arthur Conan Doyle --===============5977801081119290182==-- From drussel@gmail.com Mon Oct 20 00:30:59 2008 From: Daniel Russel To: imp-dev@salilab.org Subject: Re: [IMP-dev] [IMP-commits] r770 - trunk/doc Date: Mon, 20 Oct 2008 00:30:55 -0700 Message-ID: In-Reply-To: <48FC3321.3000903@salilab.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5640856032439637121==" --===============5640856032439637121== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > Daniel Russel wrote: >> 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) > > Exactly - that's what I say, isn't it? Maybe the text is misleading in > some way? I just say "discuss these interfaces on the imp-dev mailing > list" not the implementations. Or perhaps you're referring to a > different part of the text? The deleted part :-) I need to go to sleep. --===============5640856032439637121==--