Re: Half-baked unused features.

[David Kastrup]

Carl Sorensen <address@hidden> writes:

> On 8/15/10 9:40 AM, "David Kastrup" <address@hidden> wrote:
>
>> Carl Sorensen <address@hidden> writes:
>> 
>>> On 8/15/10 7:39 AM, "David Kastrup" <address@hidden> wrote:
>>> 
>>>> Carl Sorensen <address@hidden> writes:
>>>>> 
>>>>> 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.
>> 
>> The rebase is in conflict with the other development.  I tried that
>> first, but stopped before seriously working on the rebase conflicts
>> (experience tells me that one such conflict is followed by number of
>> conflicts in the same series, making this very tiresome since your
>> conflict resolution leads to more conflicts later).  Saving time less
>> experienced gitters would likely expend.  At the cost of making the
>> review less usable (patches won't apply to user source).
>> 
>> So up to now, just a bit more than 30 minutes of work.  "short and
>> sweet, doesn't hurt a bit".  Huh.
>
> But if you don't have a patch that applies to user source, how can you
> "remove the feature in a single commit"?  It seems that the problem
> you're having right now is one of isolating the removal of the old
> code.  Your first email said "if I remove the old code in a separate
> commit that can be reverted" or something to that effect.

The problem, as pointed out already, is that the commit affects the bulk
of the indentation in the respective functions, while changing only few
lines of substance.

And that means that it conflicts with any other ongoing work on the same
code areas.  That means that rebasing across any such other work is
going to lead to merge conflicts.

> The problem you're talking about now is a problem of isolating the
> removal, not one of waiting for review on Rietveld.

The size of the patch in lines is making the difference here.  If git
were smarter about resolving pure indentation conflicts (possibly
causing nightmares for Python addicts; but actually a useful resolution
would have to be indentation-correct in other languages as well), this
would be less of a problem.

-- 
David Kastrup