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