El 28/11/2007, a las 10:47, Junio C Hamano escribió:
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?
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
