[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: development process
From: |
Jürgen Reuter |
Subject: |
Re: development process |
Date: |
Wed, 5 Feb 2020 12:58:02 +0100 |
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
- Re: development process, (continued)
Re: development process, Joram Noeck, 2020/02/05
Re: development process,
Jürgen Reuter <=