Re: Rebase/cherry-picking idea

From: Wincent Colaiuta <win@wincent.com>
Date: Nov 28, 2007, 10:04 UTC
Message-ID: <1570EAD5-9F47-4105-B3DA-49CA6FA57369@wincent.com>
In-Reply-To: <7v1waa7lcv.fsf@gitster.siamese.dyndns.org>

El 28/11/2007, a las 10:47, Junio C Hamano escribió:

Show 11 quoted lines

> I think _GIT_FOO vs GIT_FOO is an important detail, not at all a
> bikeshed color, to make things consistent.  "export VAR=VAL" also is a
> valid concern (you said in a separate message you only know about  
> bash,
> and I later asked people if they use shells that get affected with it
> but we happily run otherwise.  I personally do not think the latter  
> is a
> problem, but since somebody already raised it as a potential issue, it
> gave us a good chance to hear from people on minority platforms, if  
> only
> to build confidence in us to use that POSIX form.

I'm still a little concerned that nobody commented when I pointed out that export VAR=VAL is used elsewhere in Git, especially in git- clone.sh, which is very commonly-used porcelain. Is it a problem?

Show 6 quoted lines

> If these details (I do not think the overridability is a minor detail,
> though) look like bikeshedding to you, that is somewhat sad.  You seem
> to be very capable of producing usable code, but these details
> (consistency and flexibility) matter for longer term stability, and I
> would really want to see capable people pay attention to such details,
> especially I sometimes fail to do so myself.

I think the *key* difference between our patches was the refactoring that you did (extracting into a separate function and at the same time making the entire message customizable rather than just the end). I think that's easily explained by the fact that as a new contributor I am pretty much always going to err on the side of the most minimal change possible (minimum number of lines, and changes kept as local as possible), even if it isn't the best or most well-engineered one. Your patch is more invasive, but it's better too, and I think it's more invasive precisely because your knowledge of the codebase and your track record gives you the confidence to make non-minimal changes when you think they're better. With time I imagine I'll evolve away from such defensive, minimal patches as the one I sent.

Cheers, Wincent