coreutils
[Top][All Lists]
Advanced

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

Re: [PATCH 00/17] De-recursion for the 'tests' subdirectory.


From: Jim Meyering
Subject: Re: [PATCH 00/17] De-recursion for the 'tests' subdirectory.
Date: Wed, 05 Sep 2012 08:49:04 +0200

Stefano Lattarini wrote:
> On 09/04/2012 06:03 PM, Jim Meyering wrote:
>> Stefano Lattarini wrote:
>>> On 09/04/2012 05:25 PM, Jim Meyering wrote:
>>>>
>>>> Hi Stefano,
>>>>
>>>> I'm done reviewing.
>>>> Here's the 17-cset series I'm ready to push.
>>
>> [For anyone wondering about the message to which Stefano has
>> just replied, it was delayed due to size >100KiB, then rejected, since
>> the only one who really needed it was Stefano, and he was a recipient. ]
>>
>>> I'll (re-)review it this evening, if you can wait.  Otherwise, feel
>>> free to just push it -- I trust you enough anyway :-)
>>
>> I can wait, but do not expect to get back to it until tomorrow.
>> I've re-inserted this patch:
>>
>>     tests: more resilient about tainted absolute srcdir path
>>
> Find my final nits below.
>
> Thanks,
>   Stefano
>
> -*-*-*-
>
>> [PATCH 01/17] build: use 'check-local' to extend the 'check' target
>>
>> * tests/Makefile.am (check-local): Here, by making this depend
>> on 'vc_exe_in_TESTS' ...
>> (check): ... rather than this.  While the old usage worked, it relied
>> on an implementation detail rather than on documented behavior.
>> * src/local.mk (check-local): Similarly, make this depend on
>> 'check-README' and 'check-duplicate-no-install' ...
>> (check): ... rather than on this.
>>
> s/rather than this/rather than making this depend on them/
>
>> [PATCH 02/17] maint: avoid parsing of Makefile.am from vc_exe_in_TESTS
>>
>> * tests/Makefile.am (TESTS): Rename ...
>> (all_tests): ... like this, so that we'll still be able to know the
>> complete list of our tests even if the user override
>>
> s/override/overrides/
>
>> TESTS from the command line (which he's allowed to do by the test
>> harness API).
>>
>> [SNIP]
>>
>> +# Indirections required so that we'll still be able to know the
>> +# complete list of our tests even if the user override TESTS from the
>>
> s/override TESTS/overrides TESTS/
>
>> +# command line (which he's allowed to do by the test harness API).
>> +TESTS = $(all_tests)
>> +root_tests = $(all_root_tests)
>> +
>>
>> -# Ensure that all version-controlled executable files are listed
>> +# Ensure that all version-controlled filestable files are listed
>>  # in $(all_tests).
>>
> Uh?  I've introduced a botched comment.  It should read:
>
>     Ensure that all version-controlled files in 'tests/' that have a
>     suffix specified in $(TEST_EXTENSIONS) are listed in $(all_tests).
>
> Sorry for the confusion.

I've used the following instead.  In English, I find that writing
about "each" generally works better than writing about "all":

    # Ensure that each version-controlled file in 'tests/' with a suffix
    # specified in $(TEST_EXTENSIONS) is listed in $(all_tests).

Running "make distcheck" now.



reply via email to

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