Re: Half-baked unused features.

[Carl Sorensen]

On 8/15/10 7:39 AM, "David Kastrup" <address@hidden> wrote:

> Carl Sorensen <address@hidden> writes:
> 
>> On 8/15/10 6:48 AM, "David Kastrup" <address@hidden> wrote:
>> 
>> 
>> 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.

No, you said it was all in one commit.  So you have a branch with that
commit and you keep rebasing it.  It's quite easy to do.  And you don't have
to eliminate the change from the ongoing development in the related area; if
you're confident it's worth eliminating then eliminate it in your
development work.

And if it's really not used, then removing it will have no effect whatsoever
on your downstream stuff.

I don't think I've *ever* seen anyone propose modifications in bitrotted
stuff to be removed.  I think your argument doesn't match with the reality
of the situation.

> 
> 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.

When uploading patches to Rietveld one can choose whatever commit is desired
as the reference for the upload, so I think that overlapping patches can be
handled without too much difficulty.
> 
> 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.

I'm not the final arbiter here, but I don't think that we should move to a
mode that says "Any developer who wants to can remove code without getting
approval."

I don't think it's at all unreasonable to ask you to post a patch showing
what you intend to remove.

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

<tongue-mostly-in-cheek>
I learned long ago (I have 7 children) that you can't stop children from
whining.  You just have to ignore them when they whine so they can't see any
benefit from whining.  Otherwise, whining becomes a never-ending tactic.
So, if you ask me, the answer would be "can't change anything in response to
David's whining." 
</tongue-in-cheek>

Thanks,

Carl