Re: Keep yaffut from attempting to demangle. (issue 5375051)

[Graham Percival]

On Fri, Nov 11, 2011 at 02:42:55PM +0100, Jan Nieuwenhuizen wrote:
> David Kastrup writes:
> 
> > The patch in its current state does not affect a working test suite, and
> > it lets "make check" finish with slightly less readable output when
> > cxxabi.h breaks g++ 4.6.  So it is _strictly_ an improvement.
> 
> Then commit it already!

Classy.

(the below is NOT directed at David, who is an absolutely
fantastic developer)


"one programmer says it's good.  So let's just bypass the review
process and commit it."

Might I remind you that almost everybody wants to have stable
release more often?  And that the biggest cause of not having
stable releases are regressions?  And that the best time to fix
regressions are *before* they happen?  And that code reviews have
been found to be the best single factor for catching bugs?  And
that nobody (apart from maybe you) actually knows what yaffut is
doing or how it works?

David is good.  Really good.  But he's also human.  Maybe he added
an extra semicolon somewhere that makes the code look ok, but blow
up in certain circumstances.  (I've spent at least 20 hours of my
life on semicolon bugs)
Maybe his changes work great on his machine, but they'll fail when
using a different version of gcc.  Maybe he just has some variable
names that are unclear, or maybe a one-line comment would clarify
something... I mean, after staring at his change to yaffut.hh for
a while, I gather that it's constructing a name for a type... but
if I'd seen
  // construct a name for a type of variable
at the top of the demangle() function, I wouldn't have to spend a
minute or two puzzling through stuff like
  std::string name (ptr ? ptr : "", ptr ? strlen (ptr) : 0);

Of course the lack of a comment like that isn't David's fault;
it's the fault of whoever wrote that code in the first place.  But
this would be a good opportunity to clarify that, for future
programmers.


Not all programmers are as good as you and Han-Wen.  But sadly,
lilypond is being maintained by us stupid programmers.
(although we *do* still have some genius programmers as well, so
don't think that I'm insulting all lilypond developers who read
this.  I'm only claiming that *I'm* a stupid programmer, not you)

If lilypond was alpha quality, it wouldn't matter if things
magically broke.  It's alpha, who cares!

If lilypond was a commercial software product, it wouldn't matter
(much) if we had difficult-to-read code.  If the code can only be
read by developers with 5 years of C++ experience, so what?  ok,
those developers might cost twice as much as junior programmers,
but that's just a business decision -- how much time should the
current programmers spend writing clear code, vs. how much do we
want to pay for hiring new developers.

Unfortunately, most people expect (or at least want) lilypond to
be stable, and we have no budget for hiring experienced
developers.


The 48-hour "patch countdown" is designed to give people a chance
to address both problems -- does the code work, and is it
*obvious* how the code works.  Granted, we rarely take advantage
of the review period to actually discuss what the code is doing
and request more comments... but this *does* happen occasionally,
and I definitely think it's worth keeping the *opportunity* for
idiots like me to ask for help in understanding the code.

- Graham