Elia Pinto <gitter.spi...@gmail.com> writes:

> I have no problems rerolling  this simple patch, but i need to know
> what is the (git) right style in this case.

If I were doing this...

diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index 66ce4b0..c1d0b23 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -8,7 +8,7 @@ This test verifies the basic operation of the merge, pull, add
 and split subcommands of git subtree.
 '
 
-export TEST_DIRECTORY=$(pwd)/../../../t
+TEST_DIRECTORY=$(pwd)/../../../t && export TEST_DIRECTORY

... I'd say I would make this two lines without &&, i.e.

        TEST_DIRECTORY=$(pwd)/../../../t
        export TEST_DIRECTORY

because we are not doing anything about a failure of $(pwd), other
than skipping the export.  If we were failing the entire thing, it
would be a different story, but at this point of the test, it is
unreasonable to do something like:

        TEST_DIRECTORY=$(pwd)/../../../t &&
        export TEST_DIRECTORY || exit $?

anyway.
 
diff --git a/git-remote-testgit.sh b/git-remote-testgit.sh
index 1c006a0..9d1ce76 100755
--- a/git-remote-testgit.sh
+++ b/git-remote-testgit.sh
@@ -13,7 +13,7 @@ refspec="${GIT_REMOTE_TESTGIT_REFSPEC-$default_refspec}"
 
 test -z "$refspec" && prefix="refs"
 
-export GIT_DIR="$url/.git"
+GIT_DIR="$url/.git" && export GIT_DIR
 
 force=

Similarly, as we do not do anything if the assignment to GIT_DIR
fails here, just like we ignore any failure from assignment to
force,

    GIT_DIR="$url/.git"
    export GIT_DIR

would be sufficient.

The logic before the "mkdir -p" of this script may want to be
cleaned up (can you tell how "if $refspec is empty, set prefix=refs"
makes any sense, and what its implications are, without thinking for
more than 10 seconds?), but that is clearly a separate topic [*1*].


diff --git a/git-stash.sh b/git-stash.sh
index 4798bcf..0bb1af8 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -94,7 +94,7 @@ create_stash () {
                # ease of unpacking later.
                u_commit=$(
                        untracked_files | (
-                               export GIT_INDEX_FILE="$TMPindex"
+                               GIT_INDEX_FILE="$TMPindex" && export 
GIT_INDEX_FILE &&
                                rm -f "$TMPindex" &&
                                git update-index -z --add --remove --stdin &&
                                u_tree=$(git write-tree) &&

This one does care about failures.  I'd rather have these on
separate lines, i.e.

                                GIT_INDEX_FILE="$TMPindex" &&
                                export GIT_INDEX_FILE &&
                                rm -f "$TMPindex" &&
                                ...


[Footnote]

*1* Perhaps something along these lines with the exact placements of
    blank lines to group similar things together:

        alias=$1 url=$2

        if test -z "${GIT_REMOTE_TESTGIT_REFSPEC-notEmpty}"
        then
                # only if it is explicitly set to an empty string...
                prefix=refs
        else
                prefix="refs/testgit/$alias"
        fi
        dir="$GIT_DIR/testgit/$alias"
        default_refspec="refs/heads/*:${prefix}/heads/*"
        refspec="${GIT_REMOTE_TESTGIT_REFSPEC-$default_refspec}"
        force=

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

Reply via email to