automake
[Top][All Lists]
Advanced

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

Re: Fixing plural case in Banner


From: Ralf Wildenhues
Subject: Re: Fixing plural case in Banner
Date: Thu, 9 Oct 2008 22:38:31 +0200
User-agent: Mutt/1.5.18 (2008-05-17)

Hi William,

Thanks for the patch.  Please send further patches to the
automake-patches list.

I found one really minor nit, at a glance:

> --- a/lib/am/check.am
> +++ b/lib/am/check.am
> @@ -86,24 +86,35 @@ check-TESTS: $(TESTS)
>           echo "$${col}$$res$${std}: $$tst"; \
>         done; \
>  ## Prepare the banner
> +       if test "$$all" -eq 1; then \
> +         tests="test"; \
> +         All=""; \
> +       else \
> +         tests="tests"; \
> +         All="All"; \

How about making that "All " ...

> +       fi; \
>         if test "$$failed" -eq 0; then \
>           if test "$$xfail" -eq 0; then \
> -           banner="All $$all tests passed"; \
> +           banner="$$All $$all $$tests passed"; \

... and removing the space after $$All here, so there is no leading
white space (once more instance below)?

>           else \
> -           banner="All $$all tests behaved as expected ($$xfail expected 
> failures)"; \
> +           banner="$$All $$all $$tests behaved as expected ($$xfail expected 
> failures)"; \
>           fi; \
>         else \

Also, please note that we have a policy that bugs fixed need tests to
ensure that the bugs remain fixed.  Would you be willing to write such
a test?  (No problem if not, I will do it then.)

If you do, please note that it's probably large enough then that we will
need a copyright assignment for changes from you (more off-list if you
are still interested).

Cheers, and thanks,
Ralf




reply via email to

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