lilypond-devel
[Top][All Lists]
Advanced

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

Re: Sketch of not remaking html files (issue 5498093)


From: graham
Subject: Re: Sketch of not remaking html files (issue 5498093)
Date: Mon, 02 Jan 2012 21:41:23 +0000

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.

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)

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!)

http://codereview.appspot.com/5498093/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]