On Jul 26, 2011, at 12:05 PM, David Kastrup wrote: > "m...@apollinemike.com" <m...@apollinemike.com> writes: > >> On Jul 26, 2011, at 11:22 AM, Jan Nieuwenhuizen wrote: >> >>> David Kastrup writes: >>> >>>> The overall code makes obvious that this has been created by a >>>> comparative novice to the programming languages and data structures of >>>> Lilypond. He has been doing his best. >>> >>> I think this patch should be reverted, moved to Rietveld, and worked >>> on. > > Possibly. > >> The reason for my pow (2.0,...) is because the push broke make and my >> fix passed regtests. > > My point was that a more experienced person patched something up > afterwards, and the patch was exclusively for passing a regtest, _kept_ > the problematic real arithmetic and other stuff in the surroundings. > > And no: the regtest _still_ gives out warnings for unknown glyphs. If > people ignore warnings in regtests, we will be forced to turn warnings > into errors. >
This is my fault. I don't know why I missed it these warnings in the side-by-side comparison, but I won't let this slip again. >> Perhaps this would be a good case study. > > The more interesting question is just how common the circumstances are > under which this happens. > > Perhaps a minimal measure of sanity would be if a patch countdown > without code review was only started when the author of the patch says > "I feel reasonably confident that this not just works, but is good". > > In git, there is the "formal" sanctification of "Signed-off-by". > Perhaps we should not start a patch countdown on any patch that has not > been signed off by anybody? > I completely agree. This is a condition I impose on myself, and I would not at all mind it to be policy (at least for newer developers). >> 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. > >> Did it pass regtests? > > Sure, but with quite suspicious warnings relevant to the test case. > >> Is there any chance that a patch could make it through full review and >> still not build on all Unix-based platforms? > > Of course. > >> Normally, before making any change I post a patch for review, but as >> this seemed fairly urgent, I sent an e-mail to devel and "fixed" the >> issue so that the current master would build. > > Your patch was an improvement in some sense, but it also was a missed > opportunity for improvement, masking a larger problem. > I see what you mean - I had not been operating under the assumption that the original was problematic, but was rather in "oh-crap" mode with regards to current master. Cheers, MS _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
