On 11/11/2010 01:39 PM, Matthias Bolte wrote: > Tweak pre tags in docs/hacking.html.in to achieve proper > indentation of their plaintext representation. > > Also use more b/i/code tags in docs/hacking.html.in. > --- > HACKING | 602 +++++++++++++++++++++++++++++--------------------- A lot of this diff is whitespace; the rest looks like it is pulling in missed bits. Overall, it looks pretty decent; and it's certainly more maintainable! > Makefile.am | 8 + > docs/hacking.html.in | 351 ++++++++++++++++-------------- Would you mind splitting this into two patches? One that cleans up just hacking.html.in, and the rest that covers going from a clean source file into the new generated HACKING file. > +++ b/Makefile.am > @@ -48,6 +48,8 @@ EXTRA_DIST = \ > pkgconfigdir = $(libdir)/pkgconfig > pkgconfig_DATA = libvirt.pc > > +all: NEWS HACKING Hmm. This means that everyone will attempt to build NEWS and HACKING, even if they don't have the prerequisite tools. > + > NEWS: $(top_srcdir)/docs/news.xsl $(top_srcdir)/docs/news.html.in > -@(if [ -x $(XSLTPROC) ] ; then \ This says that rebuilding NEWS is not fatal if you lack the prereq tools, but that means the timestamp is never updated, which means every run of 'make' will be that much slower. It might be better to have _only_ 'make syntax-check' depend on these two generated files, or even to leave things like they are for NEWS, where you have to do a manual 'make NEWS' when it is really time to do the update. > $(XSLTPROC) --nonet $(top_srcdir)/docs/news.xsl \ > @@ -56,6 +58,12 @@ NEWS: $(top_srcdir)/docs/news.xsl $(top_srcdir)/docs/news.html.in > | perl -pe 's/[ \t]+$$//' \ > > $@-t && mv $@-t $@ ; fi ); > > +HACKING: $(top_srcdir)/docs/hacking1.xsl $(top_srcdir)/docs/hacking2.xsl \ Hmm. NEWS is distributed (by virtue of automake), HACKING is not (automake doesn't auto-distribute it, and we didn't list it in Makefile.am). NEWS is not stored in git, HACKING currently is. Should HACKING be removed from git? With NEWS, the most likely audience is the end user from a tarball, at which point NEWS already exists. And that means we have a pre-existing bug - since NEWS is distributed, it should be built in $(srcdir) not $(builddir). As long as you do an in-tree build, they are one and the same, but a developer doing a VPATH build gets NEWS generated in the wrong directory given the rule above. With HACKING, the most likely audience is a developer, working from git (in fact, the reason we don't distribute it is that we assume someone working from a tarball rather than from git probably won't care about the contents of HACKING). On the other hand, a developer that does a fresh clone of libvirt.git will want instructions on how to bootstrap, and if those instructions are hidden in docs/hacking.html.in until after the first 'autogen.sh && make', then they aren't too helpful. That argues for keeping HACKING still in git. If HACKING stays in git, then it must also reside in $(srcdir) rather than $(builddir), which means this rule needs a bit of tweaking to be VPATH-friendly. > + $(top_srcdir)/docs/wrapstring.xsl $(top_srcdir)/docs/hacking.html.in > + -@(if [ -x $(XSLTPROC) ] ; then \ > + $(XSLTPROC) --nonet $(top_srcdir)/docs/hacking1.xsl $(top_srcdir)/docs/hacking.html.in | \ > + $(XSLTPROC) --nonet $(top_srcdir)/docs/hacking2.xsl - \ > + > $@-t && mv $@-t $@ ; fi ); > > rpm: clean > @(unset CDPATH ; $(MAKE) dist && rpmbuild -ta $(distdir).tar.gz) > diff --git a/docs/hacking.html.in b/docs/hacking.html.in > index bd8b443..88db01a 100644 > --- a/docs/hacking.html.in > <li><p>Post patches in unified diff format. A command similar to this > should work:</p> > - <pre> > - diff -urp libvirt.orig/ libvirt.modified/ > libvirt-myfeature.patch</pre> > +<pre> > + diff -urp libvirt.orig/ libvirt.modified/ > libvirt-myfeature.patch > +</pre> Changes like this make sense, since whitespace is significant within a <pre> block. > --- /dev/null > +++ b/docs/hacking1.xsl > @@ -0,0 +1,28 @@ > +<?xml version="1.0"?> > +<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform";> > + > +<xsl:output method="xml" encoding="UTF-8" indent="no"/> Here, I'll have to take your word that this works; of course, I plan to test it by applying the patch and running 'make HACKING', but I haven't got that far yet (wanted to get this email sent first). -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 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