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

quoted text
> 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?

quoted text
> 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

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html