----- Original Message -----
From: <gra...@percival-music.ca>
To: <philehol...@googlemail.com>; <perciv...@gmail.com>
Cc: <lilypond-devel@gnu.org>; <re...@codereview-hr.appspotmail.com>
Sent: Monday, January 02, 2012 9:41 PM
Subject: Re: Sketch of not remaking html files (issue 5498093)
a few minor quibbles; I'm on a ferry right now so I can't test a full
doc build.
http://codereview.appspot.com/5498093/diff/1/GNUmakefile.in
File GNUmakefile.in (right):
http://codereview.appspot.com/5498093/diff/1/GNUmakefile.in#newcode125
GNUmakefile.in:125: # find $(outdir) -name '*-root' | xargs rm -rf
please either delete the line entirely, or add a comment explaining why
you've commented-out that line. Otherwise I fear that this may become
another time bomb that may confuse future people working on the build
process.
I commented it out for my initial trial - easier to put it back if there's a
problem. Looks like deleting the line is the best option.
http://codereview.appspot.com/5498093/diff/1/python/auxiliar/postprocess_html.py
File python/auxiliar/postprocess_html.py (right):
http://codereview.appspot.com/5498093/diff/1/python/auxiliar/postprocess_html.py#newcode353
python/auxiliar/postprocess_html.py:353: SourceTime =
os.path.getmtime(file_name)
there's an extremely strong convention in python programming to use
lower-case and underscores, i.e.
source_time = os.path.getmtime(file_name)
No problems. I'm just a Camel caser.
http://codereview.appspot.com/5498093/diff/1/scripts/build/www_post.py
File scripts/build/www_post.py (right):
http://codereview.appspot.com/5498093/diff/1/scripts/build/www_post.py#newcode76
scripts/build/www_post.py:76: sys.exc_clear()
why do you want to catch this exception? Surely if mkdir fails, we want
to display the error message on the command-line?
maybe do something like
if not os.path.isdir(out_root):
os.mkdir (out_root)
instead?
(I mean, what if the exception that was raised was "you have no disk
space left on this drive", or something more serious than "that
directory already exists" ? -- and yes, I've occasionally tried to
build lilypond on a drive that wasn't big enough, so this isn't just a
theoretical concern!)
I spent a while researching online as to how to create a directory if it
didn't already exist, and consensus was that catching the exception was the
best option. There's some sort of issues with race conditions over checking
whether it exist before creating it - although thinking about this, it's
possible that this would only occur if more than one process was trying to
create it at the same time, but anyway.... Probably the very best way is
to catch the specific exception of directory exists and throw any others.
Having said that, it's almost certainly theoretical. If mkdir fails because
of a disk problem, something else will blow pretty soon.
http://codereview.appspot.com/5498093/
I'll have a think and put up a new patch.
--
Phil Holmes
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
