Re: development process

[Jürgen Reuter]

   Hi all,

   I fully agree with Han-Wen.  I also could now and then (maybe once a
   week) set aside 2-3 hours work for lily.  But the current development
   process really makes it hard for me to keep up submitting a patch (as
   part of currently 11 commits (mostly small to medium ones), partially
   with dependencies, that have been lying around locally on my machine
   for at least ~3-4 years), as I can't just "git push" my 11 commits
   after these 2-3 hours of work, and then, in my next available slot,
   maybe a week or two weeks later, look at code reviews / comments from
   other people, do a "git fetch" / "git rebase", consider the code
   reviewer's comments, and submit an updated version of the 11 commits.
   With Gerrit / Jenkins (which I use at work, currently version 2.14 with
   old GUI), this is really easy possible (without needing a seperate
   branch for each of the commits, which is hardly possible if there are
   dependencies), and use of Gerrit well established process, such I can
   switch to other tasks and return back to these commits even after a
   week or two, without any hassle.

   With the current setup with Retvield / Subversion Bug Tracking / etc.
   however, I do not see how I can handle this complex process, given a
   maximum of 2-3 hours (if at all) per week, if just the process itself
   for keeping up with the status after a week or two of inactivity
   probably will already consume these 2-3 hours, as I fear.

   So, my vote is +1 for a Gerrit based solution, maybe with Jenkins as
   verification build server.

   Greetings,
   Jürgen


   On Tue, Feb 4, 2020 at 10:57 PM Han-Wen Nienhuys <address@hidden>
   wrote:

     As promised in several code reviews, here my take on the current
     development process. I wrote it as a google doc first, so you can
     also go
     here
     [1]https://docs.google.com/document/d/1BSffzjiQKMTTmr988ezMbsJy
     fwT9S-rxGRbYSBTv3PM/edit
     for
     inline comments.
     Context:
        -
        [2]https://codereview.appspot.com/561390043/#msg32
        -
        [3]https://docs.google.com/document/d/1zYwygWEKJt21rl-bKL41Ipj
     FmiBjlfob6eS63jd-62I/edit#
     My experiences with the current Lilypond development process.
     For context, I have a busy daytime job. I work 80% so I can set
     aside a
     couple of hours of concentrated hacking time on Friday. When I am in
     my
     element, I can easily churn out a dozen commits in a day. Those
     often touch
     the same files, because a fix often needs a preparatory cleanup
     (“dependent
     changes”).
     My annoyances so far are especially with the review/countdown
     process :
        -
        Rietveld + git-cl doesn’t support dependent changes. So to make
     do, I
        explode my series of changes in individual changes to be reviewed
     (I
        currently have 34 branches each with a different commit so git-cl
     can match
        up branch names and reviews). For dependent changes, I have to
     shepherd the
        base change through review, wait for it to be submitted, and then
     rebase
        the next change in the series on origin/master.
        -
        Because the review happens on the split-out patches, I now switch
     back
        and forth between 34 different versions of LilyPond. Every time I
     address a
        review comment, I go through a lengthy recompile. The large
     number of
        changes clogs up my git branch view.
        -
        The countdown introduces an extra delay of 2 days in this already
        cumbersome process.
        -
        The review process leaves changes in an unclear state. If Werner
     says
        LGTM, but then Dan and David have complaints, do I have to
     address the
        comments, or is change already OK to go in?
        -
        We track the status of each review in a different system (the bug
        tracker), but the two aren’t linked in an obvious way: I can’t
     navigate
        from the review to the tracker, for example. Some things (eg.
     LGTM) are to
        be done in the review tool, but some other things should be in
     the
        bugtracker.
        -
        Using the bug tracker to track reviews is new to me. It is common
     for a
        bug to need multiple changes to be fixed. It also adds another
     hurdle to
        new developers (setting a sourceforge account and getting that
     added to the
        project).
        -
        I have to keep track of my own dashboard: once changes are
     pushed, I
        have to look them up in Rietveld to click the ‘close’ button.
        -
        Rietveld and my local commits are not linked. If I change my
     commits, I
        update my commit message. I have to copy those changes out to
     Rietveld by
        hand, and it took me quite a while to find the right button
     (“edit issue”,
        somewhere in the corner).
     Some of my complaints come from having to manage a plethora of
     changes, but
     I suspect the process will trip new contributors up too, to note:
        -
        Seeing your code submitted to a project is what makes coders
     tick. It is
        the Dopamine hit Facebook exploits so well, and so should we. The
     key to
        exploiting it is instant gratification, so we should get rid of
     artificial
        delays
        -
        We use “we’ll push if there are no complaints” for contributions.
     I
        think this is harmful to contributors, because it doesn’t give
     contributors
        a clear feedback mechanism if they should continue down a path.
     It is
        harmful to the project, because we can end up adopting code that
     we don’t
        understand.
        -
        The whole constellation of tools and processes (bug tracker for
     managing
        patch review, patch testing that seems to involve humans,
     Rietveld for
        patches, countdown, then push to staging) is odd and different
     from
        everything else. For contributing, you need to be very passionate
     about
        music typography, because otherwise, you won’t have the energy to
     invest in
        how the process works.
     David uses a patch <[4]https://debbugs.gnu.org/cgi/b
     ugreport.cgi?bug=17474> he
     made to GUILE as an example that code can be stuck in a limbo. I
     disagree
     with this assessment. To me it shows the GUILE community considering
     and
     then rejecting a patch (comment #26 and #40). I imagine that David
     was not
     happy about this particular decision, but I see a process working as
     expected. If anything, it took too long and was not explicit enough
     in
     rejecting the patch. Also, in cases like these, one would normally
     build
     consensus about the plan before going off and writing a patch.
     David suggests that I like getting patches in by fiat/countdown, but
     I
     disagree. If you look at the past 2 weeks, you can see that I have
     actually
     tried to address all code review comments so far, and again, it is
     more
     important for the project to have explicit consensus than that
     individual
     contributors that go off in possibly conflicting directions.
     Here is what a development process should look to me
        -
        Uncontroversial changes can be submitted immediately after a
     maintainer
        has LGTM’d it, and automated tests pass. Merging such a change
     should be an
        effortless single click, and should not involve mucking with the
     git
        command line. Because submitting requires no effort, we won’t
     have patches
        stuck because we’re too lazy to do the work of merging them.
        -
        There is a central place where I can see all my outstanding code
        reviews, my own, but also from other people. This means that I
     can do
        reviews if I have some time left.
        -
        The review tool supports chains of commits natively.
        -
        The review tool supports painless reverts. When it is easy to fix
     up
        mistakes, contributors will feel less afraid to make changes.
        -
        Right now, results from build/test are available during review.
     This is
        a good thing, and we should keep it.
        -
        There is no “lack of feedback means it goes in”. By accepting a
     code
        contribution we implicitly take on the duty to maintain it and
     fix bugs in
        it. If no maintainer can be convinced a piece of code is worth
     taking it
        on, then that is a long-term maintenance risk, and it should be
     rejected.
        -
        Coordinated, larger efforts across the code base should start out
     with a
        discussion. The mailing list could work here, but I find
     discussion in an
        issue tracker is often easier to manage, because it is easier to
     search,
        and by its nature, discussion in an issue tracker drifts less
     off-topic.
        -
        We have a patch meister whose job it is to hound the project
     maintainer
        to look at patches that don’t find traction or otherwise.
     --
     Han-Wen Nienhuys - [5]address@hidden -
     [6]http://www.xs4all.nl/~hanwen

References

   1. 
https://docs.google.com/document/d/1BSffzjiQKMTTmr988ezMbsJyfwT9S-rxGRbYSBTv3PM/edit
   2. https://codereview.appspot.com/561390043/#msg32
   3. 
https://docs.google.com/document/d/1zYwygWEKJt21rl-bKL41IpjFmiBjlfob6eS63jd-62I/edit
   4. https://debbugs.gnu.org/cgi/bugreport.cgi?bug=17474
   5. mailto:address@hidden
   6. http://www.xs4all.nl/~hanwen