Re: [PATCH v2 1/2] version-gen: cleanup

> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> +describe () {
>> +     VN=$(git describe --match "v[0-9]*" --abbrev=7 HEAD 2>/dev/null) || return 1
>>       case "$VN" in
>> +     *$LF*)
>> +             return 1
>> +             ;;
>>       v[0-9]*)
>>               git update-index -q --refresh
>>               test -z "$(git diff-index --name-only HEAD --)" ||
>> +             VN="$VN-dirty"
>> +             return 0
>> +             ;;
>>       esac
>> +}
>> +
>> +# First see if there is a version file (included in release tarballs),
>> +# then try 'git describe', then default.
>> +if test -f version
>> +then
>> +     VN=$(cat version) || VN="$DEF_VER"
>> +elif test -d ${GIT_DIR:-.git} -o -f .git && describe
>>  then
>
> Makes sense; using a helper function makes the primary logic easier
> to read.
>
>> -test "$VN" = "$VC" || {
>> -     echo >&2 "GIT_VERSION = $VN"
>> -     echo "GIT_VERSION = $VN" >$GVF
>> -}
>> -
>> -
>> +test "$VN" = "$VC" && exit
>> +echo >&2 "GIT_VERSION = $VN"
>> +echo "GIT_VERSION = $VN" >$GVF
>
> The point of this part is "if the version file is already up to
> date, do not rewrite it only to smudge the mtime to confuse make",
> so it would be much easier to read to write it like so:
>
>         if test "$VN" != "$VC"
>         then
>                 ... two echoes ...
>         fi
>
> compared to "exiting in the middle" which is harder to follow,
> especially if we consider that we may have to grow the remaining two
> lines in the future.