On 8/15/10 7:39 AM, "David Kastrup" <[email protected]> wrote: > Carl Sorensen <[email protected]> writes: > >> On 8/15/10 6:48 AM, "David Kastrup" <[email protected]> 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 _______________________________________________ lilypond-devel mailing list [email protected] http://lists.gnu.org/mailman/listinfo/lilypond-devel
