Thanks for the review!
>>>>> "Mark" == Mark J Nelson <Mark.J.Nelson at Sun.COM> writes:
Mark> Should the devref repo have an explicit .hgignore file for
Mark> rawchunks and cleanchunks dirs?
Well, it means that changes to the temporary directory names will
require makefile and .hgignore updates. Given the size of the
workspace, I'm inclined to think it's not worth doing, though I'm not
adamantly opposed.
Mark> 99-100: this can go away now
Fixed.
Mark> 105: without the $(ALLFILES) dependency, would this be better as a
Mark> .xml.html suffix rule?
I tried that and got this error:
make: Fatal error: Don't know how to make target `devref.html'
I don't know why, and I can't say it's high on my list of things to look
into at the moment... :-/
Mark> 108: It seems a little bit confusing for "all" to not depend on
Mark> "chunks." Isn't building this for posting a common scenario?
Good point. In fact, that's probably the primary scenario, so I've
changed the default target to be (just) "chunks". People that want a
single HTML file will need to do "make devref.html".
Mark> 116/123: the $(CLEANCHUNKDIR) dependency seems like it might be a
Mark> little bit misplaced, in that it's part of the dependency chain of
Mark> rawchunks instead of $(ONCHUNKS). Seems like there could be a
Mark> race condition for a fast, parallel build, where something could
Mark> fire the actions on 142 (and therefore 136) before 123?
This is serial make (no .PARALLEL target). IMO it's not big enough to
be worth trying to parallelize.
Mark> ...but now that I look at it a little bit more, the rawchunks
Mark> stuff is strictly an intermediate product. Instead of having raw
Mark> and clean chunk dirs, why not use xargs to invoke fixup.py on each
Mark> chunk in turn at the end of the rule on line 117-118?
I don't understand what you're suggesting. xsltproc runs once and
produces a bunch of HTML files whose contents and names are close to,
but not exactly, what we want. fixup.py cleans it up *except* for
combining ch03b1.html and ch03b2.html into ch03b.html, and renaming
apa.html to glossary.html.
Would the flow be easier to understand if I used fewer intermediate
targets and more rules per target? That might also let me get rid of
one intermediate directory.
Mark> (Generally, a $(TGTDIR)/% target with a dynamically created
Mark> $(TGTDIR) should depend on both $(TGTDIR) and $(SRCDIR)/%.)
I've come to the conclusion that this causes more trouble than it's
worth. See item 3 at http://bugs.opensolaris.org/view_bug.do?bug_id=6541200.
Instead, there should be a separate "phase" in the makefile that ensures
the existence of target directories. That's what I've done here with
the "chunktmpdirs" target.
Mark> 125-126 and 128-129: these can be combined into a single action if
Mark> you use $@ in the rule.
Done.
Mark> The "-p" option to mkdir doesn't hurt anything, but isn't needed.
I left in the -p, in case someone decides in the future that the
temporary directories should nest further down in the directory tree.
Mark> 150: I generally don't like wildcards in clean/clobber targets, at
Mark> least not where people might be doing active work. And isn't the
Mark> "*~" an artifact of your editor of choice, rather than the build
Mark> process?
I removed the "*~". Yes, it's an artifact of my editor, though it's
also a common idiom for "make clean".
For some reason fixup.pyc is no longer being created, so I removed *.pyc
from the "clean" target, too.
I changed the *.html to
rm -f devref.html
rm -f index.html ch*.html ch*s*.html apa.html
The exact chapter and section file names depend on the content in the
XML file. Trying to keep the makefile consistent with the XML here
seems more of a hassle than it's worth.
Let me know what you think about the intermediate targets and
directories (rawchunks et al, as discussed above).
thanks,
mike
