Graham Percival <gra...@percival-music.ca> writes: > On Tue, Jul 26, 2011 at 12:05:31PM +0200, David Kastrup wrote: >> "m...@apollinemike.com" <m...@apollinemike.com> writes: > ... >> > Did 104f80daf1dab11ef5b598006e3d4be8dfbe1926 go through full review? >> > Was it verified with a full build? >> >> Very likely. This was not "gold star" quality which tends to work on >> first compilation attempt. > > This patch looks absolutely excellent to me. > > 104f80daf1dab11ef5b598006e3d4be8dfbe1926 > fixed: > http://code.google.com/p/lilypond/issues/detail?id=1655 > first patch set May 17 or so: > http://codereview.appspot.com/4543055/ > next patch set May 26 or so: > http://codereview.appspot.com/4536068/ > this second patchset had a total of: > - 6 drafts > - 9 comments not from the author > - a LGTM from Neil > - countdowns on > June 03 > http://lists.gnu.org/archive/html/lilypond-devel/2011-06/msg00036.html > June 06 > http://lists.gnu.org/archive/html/lilypond-devel/2011-06/msg00104.html > June 14 > http://lists.gnu.org/archive/html/lilypond-devel/2011-06/msg00283.html > > Speaking from an administrative perspective, this patch looked > *excellent* to push.
The term "gold star" refers to code that runs on first compilation. It is not a measure of the final quality. I was giving a subjective impression of how "natural" the code appeared to me, and the code does not exhibit the light and consistent hand of a master. It looks like the result of work rather than inspiration. And that means that I am fairly confident it _was_ verified with a full build. Likely several full builds before it compiled without errors. It has a regression test fitting it, to boot. I agree with you that from an administrative perspective, this patch could hardly have been better. From my personal perspective as code wrangler, there are a number of deficiencies in code and execution. Probably worst is that the code does not speak for itself. Often you look at code, and see what it does and why. Not here. So it needs to tell its story in comments. It doesn't. There is a lot of code in Lilypond that nonchalantly expects people to get along without commenting what it does. This is often a nuisance, but if the code is written by a master, the pain of figuring out what it does is usually tolerable. I don't feel confident understanding what this patch does and why. Masters don't grow on trees. So we have, in my view, a large discrepancy between administrative quality and code quality for this patch. And I want to _stress_ that this is _not_, I repeat _not_ the fault of the patch author. He invested more care and effort than he sees us doing. Can we do better? -- David Kastrup _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel