Alexey Shumkin <alex.crez...@gmail.com> writes:

> Function 'test_format' is become hard to read after its change in
> de6029a2d7734a93a9e27b9c4471862a47dd8123. So, make it more elegant.
> Also, change 'commit_msg' function to make it more pretty.

I do not know where you pick up these "more elegant" and "more
pretty" from, but please refrain from using _only_ such vague and
subjective phrases to describe the change in the log message.
Saying "make it <<better>> by doing X" (with various subjective
adjectives to say "better") is fine, but make sure you have "doing
X" part in the explanation.

Perhaps like this.

    Function 'test_format' has become harder to read after its
    change in de6029a2 (pretty: Add failing tests: --format output
    should honor logOutputEncoding, 2013-06-26).  Simplify it by
    moving its "should we expect it to fail?" parameter to the end.

I cannot read why you think the updated commit_msg is "more pretty"
in the message or in the patch.

> -commit_msg () {
> -     # String "initial. initial" partly in German (translated with Google 
> Translate),
> +commit_msg() {

Style.  Have SP on both sides of () in a shell function definition.

> +     # String "initial. initial" partly in German
> +     # (translated with Google Translate),
>       # encoded in UTF-8, used as a commit log message below.
>       msg=$(printf "initial. anf\303\244nglich")
>       if test -n "$1"

This is not "more pretty" but "better commented".

> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
> index 2ef96e9..73a1bdb 100755
> --- a/t/t7102-reset.sh
> +++ b/t/t7102-reset.sh
> @@ -9,15 +9,17 @@ Documented tests for git reset'
>  
>  . ./test-lib.sh
>  
> -commit_msg () {
> -     # String "modify 2nd file (changed)" partly in German(translated with 
> Google Translate),
> +commit_msg() {
> +     # String "modify 2nd file (changed)" partly in German
> +     # (translated with Google Translate),
>       # encoded in UTF-8, used as a commit log message below.
> -     msg=$(printf "modify 2nd file (ge\303\244ndert)")
> +     printf "modify 2nd file (ge\303\244ndert)" |
>       if test -n "$1"
>       then
> -             msg=$(echo $msg | iconv -f utf-8 -t $1)
> +             iconv -f utf-8 -t $1
> +     else
> +             cat
>       fi
> -     echo $msg

Is it "more pretty"?  The "we have to have cat only because we want
to pipe into a conditional" look somewhat ugly.

        msg="modify 2nd file (ge\303\244ndert)"
        if test -n "$1"
        then
                printf "$msg" | iconv -f utf-8 -t "$1"
        else
                printf "$msg"
        fi

>  }
>  
>  test_expect_success 'creating initial files and commits' '
--
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