[gmx-developers] suggestions for coding workflow to lead to efficient review

[Szilárd Páll]

Hi,

Good points! I think such a guide coding workflow guidelines would really
be useful on a wiki.

I wonder, when do you propose that devs upload changes to jenkins? To what
extent should we offer/suggest the use of jenkins and its infrsastructure
for builds during such development?
I think it would be best if people kept unstable dev branches in their
private repos until there is really a point to share and get feedback in
form of review rather than just saving time to avoid building anything else
but e.g. the single debug & no GPU & no MPI config used for development.


Cheers,

--
Szilárd

On Tue, Sep 8, 2015 at 2:20 PM, Mark Abraham <mark.j.abraham at gmail.com>
wrote:

> Hi,
>
> We've had one or two discussions recently about how to manage our code
> development with minimal friction. Often while tackling some new or changed
> functionality, it becomes clear that some piece of infrastructure should
> change as well. The cleanup/reorganization/extension is often good to
> handle separately from the new functionality, so that other code can start
> using it immediately, and the surface area of each patch stays as small as
> possible. This makes it easier to reason about each change, when writing
> and reviewing.
>
> Ideas for how to get this done:
>
> Be mentally prepared to throw work away - your first experiment, your
> first simulation and your first draft of a paper are often horrible, and
> code is no different. You often don't know what you need to do until you've
> tried to do it and/or have the requisite general experience.
>
> Make sure you have a way to run the code easily - at least so you can know
> immediately that what you've changed compiles and doesn't segfault.
>
> Edit code with only one, small objective in mind. Commit code frequently -
> saying "WIP adding xyz - compiles!" is a great commit message for your
> local development branch. "Extended XSDE to support xcf" is also great. If
> you do this, you'll end up with a series of commits and each one is
> reasonably clear about whether it is cleanup, preparatory infrastructure
> changes, or new functionality. git rebase can then let you move your
> cleanup and infrastructure patches early in your history. Your tests will
> let you re-visit points in your revised history and see that things compile.
>
> When you realize that stuff needs cleaning up before you can do what you
> want to do, stop and think about what that means for the larger objective
> that you have - maybe you should go do that cleanup and then re-write much
> of your code in that new context.
>
> Otherwise, make a work-in-progress git stash/commit/branch (whatever works
> for you), check out an early version of this branch of your development
> repo and do the cleanup, test it, etc. Then cherry-pick your old
> work-in-progress on top of it, resolve any issues.
>
> If one has failed to do any of the above, there is still the option of
> squashing work into a single git commit, looking for some logical
> sub-pieces of the work, and using
> http://stackoverflow.com/questions/1440050/how-to-split-last-commit-into-two-in-git and
> perhaps
> http://stackoverflow.com/questions/1085162/commit-only-part-of-a-file-in-git to
> separate things.
>
> Example case: GROMACS OpenCL support
>
> StreamComputing forked GROMACS release-5-0 on github (
> https://github.com/StreamComputing/gromacs), and several of their
> developers worked on adding OpenCL functionality for several months.
> GROMACS master moved on, of course. When things were approximately
> complete, we decided that much of the OpenCL development history was of no
> interest to the GROMACS repo. I merged gromacs/master into their github
> branch, and made a number of cleanup patches (e.g.
> https://github.com/StreamComputing/gromacs/commit/2884290bada2f1245ea0c05dfd278a1356a14517)
> with a view to ending up with the ability to separate two commits. One that
> didn't change GROMACS functionality, but prepared us to move away from GPU
> being synonymous with CUDA (https://gerrit.gromacs.org/#/c/4306/) so that
> other CUDA development work could proceed without much impact on the OpenCL
> review process, and then one commit that added the new functionality (
> https://gerrit.gromacs.org/#/c/4314/).
>
> In my perfect world, from the latter commit we'd also have separated some
> forcerec.cpp-related cleanup, CMake support, fastgen, JIT caching and
> twin-range support into separate commits. The usefulness of the latter
> three partitions didn't become clear to me until very late in the piece -
> when I realized that fastgen+twin-range would remove most of the need for
> JIT caching, which was problematic to implement well.
>
> None of this lunch is free, of course. The lesson to do one thing at a
> time comes hard to smart people sometimes :-)
>
> Mark
>
> --
> Gromacs Developers mailing list
>
> * Please search the archive at
> http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List before
> posting!
>
> * Can't post? Read http://www.gromacs.org/Support/Mailing_Lists
>
> * For (un)subscribe requests visit
> https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers
> or send a mail to gmx-developers-request at gromacs.org.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://maillist.sys.kth.se/pipermail/gromacs.org_gmx-developers/attachments/20150909/8c9fe2d3/attachment.html>