Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> writes:
> larsxschneider@xxxxxxxxx writes:
>
>> + if [[ "$TRAVIS_OS_NAME" = linux ]] && [[ "$CC" = gcc ]];
>
> [[ is a bashism, and doesn't bring anything here compared to the POSIX
> [ ... ], or "test" which is prefered in Git's source code.
>
> The ; or the newline is not needed either.
Honestly, I didn't know that we were even trying to be pure POSIX,
avoid bashism or GNUism, or in general to follow our shell scripting
style in the scriptlet in the .travis.yml file.
While I feel fairly strongly about keeping the generic part generic,
I am actually OK with things that are known to be used in a specific
environment to be specific to that environment.
Having said all that, if we are not benefiting from using features
beyond POSIX, then by all means we should strive to be writing our
stuff in a portable way, as we do not have firm control over from
where and to where people cut and paste code snippets.
And I do think bashism [[ ... ]] is *NOT* buying anything in this
particular case, so I do agree with you that
if test "$TRAVIS_OS_NAME" = linux && test "$CC" = gcc
then
...
or even
case "$TRAVIS_OS_NAME,$CC" in
linux,gcc)
...
is what I would have written instead if I were writing this
conditional.
If we were to shoot for "be POSIX unless we can clearly benefit from
being bash/gnu/linux specific in bash/gnu/linux specific parts", the
existing scriptlets in .travis.yml file has a few things that may
need to be cleaned up already. There are "mkdir --parents" (POSIX
spells it "-p"), "pushd/popd" and invocation of "tar" is very GNU
specific in the part that appears in the case arm for "linux".
There also are existing instances of "useless ;" that would want to
be cleaned up regardless of portability issues.
>> + then
>> + echo ""
>> + echo "------------------------------------------------------------------------" &&
>
> I usualy avoid "echo <something-starting-with-dash>" as I'm not sure how
> portable it is across variants of "echo". Maybe this one is portable
> enough, I don't know. Perhaps printf, or cat << EOF ...?
Do you even need a long divider there?
> I think it makes sense to do some lightweight checks after "make doc",
> rather than just check the return code. For example, check that a few
> generated files exist and are non-empty, like
>
> test -s Documentation/git.html &&
> test -s Documentation/git.1
Yup, or the formatter does not give new/unknown warnings.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html