Re: [PATCHv4 15/15] Replace "unset VAR" with "unset VAR;" in testsuite as per t/README

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Thanks for all the reviews.  I'll fix things up.  One question on this
last one...

On Wed, Sep 29, 2010 at 1:48 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Elijah Newren <newren@xxxxxxxxx> writes:
>
>> Acked-by: Ãvar ArnfjÃrà Bjarmason <avarab@xxxxxxxxx>
>> Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
>> ---
>> @@ -175,8 +175,8 @@ test_expect_success 'init with init.templatedir set' '
>> Â Â Â Â Â Â Â git config -f "$test_config" Âinit.templatedir "${HOME}/templatedir-source" &&
>> Â Â Â Â Â Â Â mkdir templatedir-set &&
>> Â Â Â Â Â Â Â cd templatedir-set &&
>> - Â Â Â Â Â Â unset GIT_CONFIG_NOGLOBAL &&
>> - Â Â Â Â Â Â unset GIT_TEMPLATE_DIR &&
>> + Â Â Â Â Â Â unset GIT_CONFIG_NOGLOBAL;
>> + Â Â Â Â Â Â unset GIT_TEMPLATE_DIR;
>> Â Â Â Â Â Â Â NO_SET_GIT_TEMPLATE_DIR=t &&
>> Â Â Â Â Â Â Â export NO_SET_GIT_TEMPLATE_DIR &&
>> Â Â Â Â Â Â Â git init
>> @@ -187,7 +187,7 @@ test_expect_success 'init with init.templatedir set' '
>> Âtest_expect_success 'init --bare/--shared overrides system/global config' '
>> Â Â Â (
>> Â Â Â Â Â Â Â test_config="$HOME"/.gitconfig &&
>> - Â Â Â Â Â Â unset GIT_CONFIG_NOGLOBAL &&
>> + Â Â Â Â Â Â unset GIT_CONFIG_NOGLOBAL;
>> Â Â Â Â Â Â Â git config -f "$test_config" core.bare false &&
>> Â Â Â Â Â Â Â git config -f "$test_config" core.sharedRepository 0640 &&
>> Â Â Â Â Â Â Â mkdir init-bare-shared-override &&
>> @@ -202,7 +202,7 @@ test_expect_success 'init --bare/--shared overrides system/global config' '
>> Âtest_expect_success 'init honors global core.sharedRepository' '
>> Â Â Â (
>> Â Â Â Â Â Â Â test_config="$HOME"/.gitconfig &&
>> - Â Â Â Â Â Â unset GIT_CONFIG_NOGLOBAL &&
>> + Â Â Â Â Â Â unset GIT_CONFIG_NOGLOBAL;
>> Â Â Â Â Â Â Â git config -f "$test_config" core.sharedRepository 0666 &&
>> Â Â Â Â Â Â Â mkdir shared-honor-global &&
>> Â Â Â Â Â Â Â cd shared-honor-global &&
>
> These three hunks look wrong as they break the && cascades. ÂAll others
> (including the change to 7502) look Ok.

They break the && cascades, but aren't the && cascades already
inherently broken due to the unportable return value of unset?

> Personally I do not agree that ";" at the end is explicit enough as
> t/README seems to think. ÂIf we want to be explicit, I'd say we should do
> so by saying something like:
>
> Â Â Â Âunset VAR ;# can fail

That makes sense.  Alternatively, I asked in an earlier posting of
this series whether it would it make sense to add a
  portable_unset () {
    unset $* || true
  }
helper function to test-lib, and just use it instead of unset?  That
would allow proper chaining everywhere.  Thoughts?
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]