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

[Jiang Xin <worldhello.net@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

>         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.

>  }
>
> -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

> +                               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.

> +               (
> +                       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