Re: Half-baked unused features.

[Graham Percival]

On Sun, Aug 15, 2010 at 2:39 PM, David Kastrup <address@hidden> wrote:
> Carl Sorensen <address@hidden> writes:
>
>>> What is the general stance towards cleanup (of unused dormant stuff
>>> never documented for general use) like that as long as it is contained
>>> in separate commits and not intermingled with other changes?
?>>
>>> Should it
>>> be wrapped in a full review process?
>>
>> I think so.  The full review process for removing old stuff is
>> generally very short and sweet (post the patch, somebody important
>> says OK), so I don't think it hurts a bit to do it.

<david>
we don't *have* a "full review process" in any meaningful sense of the
term.  Especially not for "cleaning up" things.

As evidence, consider:
http://codereview.appspot.com/1724041/show

- big initial patch
- lots of comments about splitting up the patch into smaller,
easily-understood portions
- contributor (an unknown person, BTW) does what we ask
- NOBODY bloody looks at it.  The reworked patch has been rotting away
for almost 2 months.

That's a huge black mark against our development process.
</david>


I haven't complained about this previously because I didn't see any
point... I mean, it's not like we have developers sitting around on
their thumbs.  We've all been working on new features or 2.14 stuff or
the like.  Also, there's 12 other patches, many from "top tier"
developers, waiting for review.  No point fussing about a relatively
low-priority patch.

Besides, I don't like complaining about anything that I'm not willing
to fix myself, and I haven't had a chance to get familiar with that
side of lilypond.


> So yes, it does hurt in my opinion.

Agreed.  IMO, as long as the cleanup patches are on their own distinct
commits, go ahead.  That said, I'm not a good authority on .c and .scm
changes to lilypond.

> Personally, I lean towards thinking that stuff that is not used within
> Lilypond, has no user-level documentation and has never been in the
> regression tests or snippets should be fair game.

I agree.  As a general rule of thumb, if the docs compile from
scratch, the regtests are not affected, and you honestly believe that
it's unused code, go ahead and remove it.  If somebody uses lots of
weird scheme stuff and really wants them to work, they should make a
regtest.

Cheers,
- Graham