Hi Hannes,
On 9/28/2010 4:52 PM, Johannes Sixt wrote:
On Dienstag, 28. September 2010, Pat Thoyts wrote:
Junio,
The msysGit tree currently tracks some 50+ patches on top of 'next'. I
have gathered 42 of these that look good to move upstream.
I've browsed through the list, and I'll annotate my opinion below. When I
say 'OK', then this means that the patch looks good, but it doesn't imply
that I have tested it.
Side-step line-ending corruption leading to t3032 failures.
This one has non-portable 'export foo=bar', but it is in a MinGW specific
path, so it should be fine. But why do we need to export GREP_OPTIONS, but
not SED_OPTIONS?
I also normally avoid unportable 'export foo=bar'. In the particular
case of GREP_OPTIONS, when commenting on my original patch submission,
Dscho suggested 'test_have_prereq MINGW && export GREP_OPTIONS=foo' so
that is the form which made it into the final patch. While considering
whether this unportable usage was worthwhile, I took into consideration
the fact that a much more senior project member preferred it, that the
MinGW-specific build environment is Bash-based, and that a couple other
test scripts (t1509, t5560) also employ this form. If preferred, the
patches can be re-submitted to avoid the unportable usage.
Regarding exporting GREP_OPTIONS but not SED_OPTIONS: grep explicitly
recognizes the GREP_OPTIONS environment variable. My original patch did
not export GREP_OPTIONS, but instead referenced it as $GREP_OPTIONS in
the grep invocation. Upon review, Dscho preferred the variable to be
exported rather than referenced manually as $GREP_OPTIONS. sed, on the
other hand, does not recognize any 'options' environment variable. It is
necessary, therefore, to interpolate $SED_OPTIONS directly during the
sed invocation. This variable is therefore local to the script and need
not be exported.
-- ES
--
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