automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 01/10] Add new tests on strictness and warnings precedence an


From: Ralf Wildenhues
Subject: Re: [PATCH 01/10] Add new tests on strictness and warnings precedence and overriding.
Date: Sun, 2 Jan 2011 15:28:39 +0100
User-agent: Mutt/1.5.20 (2010-08-04)

Hi Stefano,

I'm not sure if I'll get through the whole series today, but I gotta
start somewhere, so here we go.  Feel free to push the patch series
(as far as OKed) on a new branch based off of maint, if that is helpful
for you.  Thanks.

* address@hidden wrote on Thu, Dec 23, 2010 at 12:27:37PM CET:
> --- /dev/null
> +++ b/tests/strictness-overriding.test

How about 'strictness-override.test'?
(I'll explain in another mail why I'm keen on not too long names.)

> @@ -0,0 +1,116 @@
> +#! /bin/sh
> +# Copyright (C) 2010 Free Software Foundation, Inc.

2011.  Sorry!

> +# The strictness specified in Makefile.am:AUTOMAKE_OPTIONS should
> +# override that specified in configure.in:AM_INIT_AUTOMAKE, and both
> +# should override the strictness specified on the command line.
> +# NOTE: this current semantic might not be the best one (even if it has
> +# been in place for quite a long time); see also Automake bug #7673.

I don't think it is grammatically correct to use 'semantic' as a noun in
singular form, similar to how 'information' works; 'semantic' is only
ever used as adjective or adverb AFAIK.  Here, "the current semantics"
would be fine.

> +# Update this test if the semantic is changed.

Likewise: "the current semantics are"

> +. ./defs || Exit 1
> +
> +set -e
> +
> +# We want complete control over automake options.
> +AUTOMAKE=$original_AUTOMAKE
> +
> +cat > Makefile.am <<'END'
> +AUTOMAKE_OPTIONS =
> +END
> +
> +set_strictness()

Space before open parenthesis; several instances.

> +{
> +  set +x

Curious: why turn off tracing here?

> +  sed <$2 >$2-t -e "s|^\\(AUTOMAKE_OPTIONS\\) *=.*|\\1 = $1|" \
> +                -e "s|^\\(AM_INIT_AUTOMAKE\\).*|\\1([$1])|"
> +  mv -f $2-t $2
> +  set -x
> +  cat $2

To avoid caching please 'rm -rf autom4te.cache' here.

> +}
> +
> +ok()
> +{
> +  $AUTOMAKE -Werror $*
> +}
> +
> +ko()
> +{
> +  AUTOMAKE_fails $*
> +  grep 'required file.*README' stderr
> +}
> +
> +$ACLOCAL
> +
> +# Leave out only one of the required files, to avoid too much
> +# repetitions in the error messages.

repetition

> +touch INSTALL NEWS AUTHORS ChangeLog COPYING

Most of the above comments apply to several tests in this patch.

Thank you,
Ralf



reply via email to

[Prev in Thread] Current Thread [Next in Thread]