On Fri, Oct 18, 2013 at 1:07 PM, Jose A. Lopes <[email protected]> wrote:
> On Fri, Oct 18, 2013 at 10:51:37AM +0000, Michele Tartara wrote:
>> Checking the correctness of the NEWS file syntax is an important part of the
>> check process, but up to now it was only possible as part of a bigger set of
>> tests. This commit creates a Makefile target to run that independently.
>>
>> The developer notes are updated to document this new target.
>>
>> Signed-off-by: Michele Tartara <[email protected]>
>> ---
>> Makefile.am | 6 ++++--
>> doc/devnotes.rst | 4 ++++
>> 2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index d15854e..0bcd349 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -2027,11 +2027,13 @@ check-dirs: $(GENERATED_FILES)
>> }
>>
>> .PHONY: check-local
>> -check-local: check-dirs $(GENERATED_FILES)
>> +check-news:
>
> Isn't a dependency on $(CHECK_NEWS) or 'check-dirs' necessary ?
$(CHECK_NEWS) is distributed with the source, and isn't supposed to
change, so I don't see why a dependency would be of any use (and in
fact, it's not there in the existing code either).
check-dirs is checking that the actual structure of the directories is
as described in the Makefile. Which makes it a part (and a dependence)
of check-local, but it has no link to check-news, that only tests the
NEWS file.
> What about a dependency on the 'NEWS' file?
I don't think a dependency from the NEWS file is needed. check-news is
not building anything. It should just run the check everytime it is
invoked, no matter if the NEWS file was updated or not. So, it's not a
dependence.
But this question made me realize that check-news is actually a phony
target and should be indicated as such.
Interdiff:
diff --git a/Makefile.am b/Makefile.am
index 0bcd349..8049d6d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2026,10 +2026,11 @@ check-dirs: $(GENERATED_FILES)
test -z "$$error"; \
}
-.PHONY: check-local
+.PHONY: check-news
check-news:
RELEASE=$(PACKAGE_VERSION) $(CHECK_NEWS) < $(top_srcdir)/NEWS
+.PHONY: check-local
check-local: check-dirs check-news $(GENERATED_FILES)
$(CHECK_PYTHON_CODE) $(check_python_code)
PYTHONPATH=. $(CHECK_HEADER) $(check_python_code)
>> + RELEASE=$(PACKAGE_VERSION) $(CHECK_NEWS) < $(top_srcdir)/NEWS
>
> Isn't it safer to use 'env' ?
>
> env RELEASE=$(PACKAGE_VERSION) ...
I reused the exact same code we are using currently. Why should "env" be safer?
Thanks,
Michele
--
Google Germany GmbH
Dienerstr. 12
80331 München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
