Re: [PATCH] build: check for pod errors

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

 



On 10/26/2012 11:36 AM, Martin Kletzander wrote:
> On 10/26/2012 06:06 PM, Eric Blake wrote:
>> Patch 61299a1 fixed a long-standing pod error in the man page.
>> But we should be preventing these up front.
>>
>> * tools/Makefile.am (virt-xml-validate.1, virt-pki-validate.1)
>> (virt-host-validate.1, virt-sanlock-cleanup.8, virsh.1): Reject
>> pod conversion errors.
>> * daemon/Makefile.am ($(srcdir)/libvirtd.8.in): Likewise.
>> ---
>>
>> I tested that with this patch but not Jirka's cleanup, 'make' failed;
>> with both patches, make succeeds and there are no buggy man pages.

>>  $(srcdir)/libvirtd.8.in: libvirtd.pod.in
>> -	$(AM_V_GEN)$(POD2MAN) --name LIBVIRTD $< $@
>> +	$(AM_V_GEN)$(POD2MAN) --name LIBVIRTD $< $@ \
>> +	    && if grep 'POD ERROR' $@ ; then exit 1; fi
>>
> 
> At first I've mistaken $< for $@, but your version is correct.  But if I
> may have a suggestion, I'd do 'then rm $@; exit', so it fails also when
> you re-run make again.

Good call.

>>  virt-xml-validate.1: virt-xml-validate.in
>> -	$(AM_V_GEN)$(POD2MAN) --name VIRT-XML-VALIDATE $< $(srcdir)/$@
>> +	$(AM_V_GEN)$(POD2MAN) --name VIRT-XML-VALIDATE $< $(srcdir)/$@ \
>> +	    && if grep 'POD ERROR' $@ ; then exit 1; fi
>>
> 
> s_$@_$(srcdir)/$@_

Bah - copy-and-paste bit me, and I only tested the bug in virsh.pod so I
didn't spot it.  Then again, that's why we do reviews :)

> 
> ACK with those fixes (both with or without the "rm" added).

Fixing to add the rm, and then I'll push.  Thanks again!

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]