Re: Half-baked unused features.

[David Kastrup]

Carl Sorensen <address@hidden> writes:

> On 8/15/10 6:48 AM, "David Kastrup" <address@hidden> wrote:
>
>> Hi, I'm just trying to improve the documentation of
>> define-markup-command and define-markup-command-list.  Those commands
>> have some functionality for defining aliases to existing commands.  For
>> one thing, I have my doubts that this works properly, for another, it
>> complicates implementation and documentation (it was never documented in
>> lilypond-extending, anyhow).
>> 
>> So I am leaning towards throwing it all out as I've checked that "make
>> test-clean && make test" is not affected.
>> 
>> I'm making sure that throwing this stuff out is a separate commit from
>> improving the documentation, so it can easily be reverted iff somebody
>> considers it desirable.
>> 
>> 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?
>
> IMO, getting rid of bit-rotted code is always a good idea.
>
>> 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.

It only involves creating a separate branch, moving the change there,
removing the change from all ongoing development in related areas
(and/or postponing work on them until the review process of the bitrot
change has come to a close), creating a Rietveld issue, uploading the
changes to Rietveld, monitoring all progress on it, repeating a full
regtest for any proposed modifications and juggling with
merge/cherry-pick while doing the parallel development and so on.

So yes, it does hurt in my opinion.  And since cleanup like that comes
up usually as a side-effect of doing serious work for which one can't
sensibly maintain a Rietveld review in parallel (since we are talking
about overlapping patches which Rietveld does not handle sanely) but has
to wait for the cleanup review to complete in its own time frame, it
stops other work in progress, at a rate hardly less than a day per
cleanup in the affected area.

So I disagree with the "short and sweet" bit because we don't have the
infrastructure to do this in parallel with related work on the same
code.  In particular if that related work is progressing in a branch.

So the real question probably is more or less "What balance between
developer sanity, project policies, and developer responsibility are we
aiming for?", and likely the answer depends on the developer, too.
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.  If only to finally
convince the person who actually needs it go to the pain of completing
its implementation.

Or maybe the question is just "what's it worth to keep David from
whining?".

-- 
David Kastrup