Re: [PATCH] receive-pack: purge temporary data if no command is ready to run

From: Jiang Xin <worldhello.net@gmail.com>
Date: Jan 25, 2022, 00:22 UTC
Message-ID: <CANYiYbGJc6RStoCfJjh0Y3BzQ1QdnoN_xSFz4keYVik1dq0cnw@mail.gmail.com>
In-Reply-To: <pull.1124.git.1642987616372.gitgitgadget@gmail.com>

On Mon, Jan 24, 2022 at 11:12 PM BoJun via GitGitGadget <gitgitgadget@gmail.com> wrote:

> -mk_empty () {
> +mk_empty() {

That's wrong. We prefer a space between the function name and the parentheses for shell script, see:

 * https://github.com/git/git/blob/master/Documentation/CodingGuidelines#L138

Show 16 quoted lines

>         repo_name="$1"
>         rm -fr "$repo_name" &&
> -       mkdir "$repo_name" &&
> -       (
> -               cd "$repo_name" &&
> -               git init &&
> -               git config receive.denyCurrentBranch warn &&
> -               mv .git/hooks .git/hooks-disabled
> -       )
> +               mkdir "$repo_name" &&
> +               (
> +                       cd "$repo_name" &&
> +                               git init &&
> +                               git config receive.denyCurrentBranch warn &&
> +                               mv .git/hooks .git/hooks-disabled
> +               )

The indent your made is ugly.

Show 26 quoted lines

>  }
>
> -mk_test () {
> +mk_test() {
>         repo_name="$1"
>         shift
>
>         mk_empty "$repo_name" &&
> -       (
> -               for ref in "$@"
> -               do
> -                       git push "$repo_name" $the_first_commit:refs/$ref ||
> -                       exit
> -               done &&
> -               cd "$repo_name" &&
> -               for ref in "$@"
> -               do
> -                       echo "$the_first_commit" >expect &&
> -                       git show-ref -s --verify refs/$ref >actual &&
> -                       test_cmp expect actual ||
> -                       exit
> -               done &&
> -               git fsck --full
> -       )
> +               (
> +                       for ref in "$@"; do

Code style of the original is goold, yours is bad. See:

 * https://github.com/git/git/blob/master/Documentation/CodingGuidelines#L100

Show 6 quoted lines

> +                               done &&
> +                               git fsck --full
> +               )
>  }
>
>  mk_test_with_hooks() {

This is the place you can fix by adding a space between the function name and the parentheses.

Show 7 quoted lines

> +               (
> +                       cd "$repo_name" &&
> +                               mkdir .git/hooks &&
> +                               cd .git/hooks &&
> +                               cat >pre-receive <<-'EOF' &&
> +                                       #!/bin/sh
> +                                       cat - >>pre-receive.actual

Too deep indent. The original implementation is good, yours is bad.

>
> -for head in HEAD @
> -do
> +for head in HEAD @; do

Bad coding style, please read through the "CodingGuidelines" for bash.

-- Jiang Xin