this breaks make doc.
http://codereview.appspot.com/5521056/diff/1/Documentation/usage/lilypond-book.itely
File Documentation/usage/lilypond-book.itely (right):
http://codereview.appspot.com/5521056/diff/1/Documentation/usage/lilypond-book.itely#newcode38
LGTM
http://codereview.appspot.com/5503093/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM
http://codereview.appspot.com/5517050/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
I really don't like the hard-coded numbers, but I suppose I can't
complain -- the patch doesn't make it any worse in that respect.
http://codereview.appspot.com/5530043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
I like some of the changes here, but I have serious doubts about others.
http://codereview.appspot.com/5520056/diff/1/Documentation/notation/rhythms.itely
File Documentation/notation/rhythms.itely (right):
LGTM
I'm vaguely curious as to what's the difference between this postscript
and the regtest for postscript, but that's no reason to hold up
development. Besides, the lilypond markup is easier to understand than
the postscript, so this is a good change to make even if it wasn't a
build problem.
LGTM
http://codereview.appspot.com/5498093/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM, but are you sure that the info on the Windows download page is
good for lilypond-book, as well as for lilypond itself?
http://codereview.appspot.com/5521056/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://codereview.appspot.com/5530069/diff/1/python/book_latex.py
File python/book_latex.py (right):
http://codereview.appspot.com/5530069/diff/1/python/book_latex.py#newcode274
python/book_latex.py:274: rep['base'] = basename.replace ('\\', '/')
hmm, isn't there anything in os.path that could
this fails to compile. Removed from staging.
http://codereview.appspot.com/5503093/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM, I could kiss you for doing this work.
Please push directly to staging once Patchy is finished checking the
compile (about 30 minutes from now).
http://codereview.appspot.com/5539052/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
Looks Absolutely Fantastic To Me.
Please push directly to staging once Patchy has finished testing the
compile (about 30 minutes?)
http://codereview.appspot.com/5541050/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
LGTM
http://codereview.appspot.com/5535047/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM, please push directly to staging.
http://codereview.appspot.com/5543044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
On 2012/01/13 16:56:54, Pavel Roskin wrote:
I don't think I can close this.
That is quite correct -- James Lowe owns this issue, so only he can
close it.
http://codereview.appspot.com/5530043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://codereview.appspot.com/5504092/diff/8001/scripts/auxiliar/lily-git.tcl
File scripts/auxiliar/lily-git.tcl (right):
http://codereview.appspot.com/5504092/diff/8001/scripts/auxiliar/lily-git.tcl#newcode13
scripts/auxiliar/lily-git.tcl:13: set push_access 1
This must be set to 0.
After commenting out the git push line, and also disconnecting my
network cable, then going to push patch, I get this:
fatal: ambiguous argument 'staging..dev/local_working': unknown revision
or path not in the working tree.
Use '--' to separate paths from revisions
Look, could we just treat
http://codereview.appspot.com/5539062/diff/1/Documentation/contributor/source-code.itexi
File Documentation/contributor/source-code.itexi (right):
http://codereview.appspot.com/5539062/diff/1/Documentation/contributor/source-code.itexi#newcode450
Documentation/contributor/source-code.itexi:450:
http://codereview.appspot.com/5539062/diff/1/Documentation/contributor/source-code.itexi
File Documentation/contributor/source-code.itexi (right):
http://codereview.appspot.com/5539062/diff/1/Documentation/contributor/source-code.itexi#newcode491
Documentation/contributor/source-code.itexi:491:
LGTM
http://codereview.appspot.com/5557056/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM
http://codereview.appspot.com/5553056/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM
http://codereview.appspot.com/5504092/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM, but I can't speak to the translation stuff. Depending on what
Francisco says (or has said), you might want to split this into two
separate patches: one for non-translation stuff, and one for
translations.
http://codereview.appspot.com/5562043/
LGTM, please push directly to staging.
As a general rule, any spelling fixes like this can be pushed directly
(as long as they don't involve translations).
http://codereview.appspot.com/5561053/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
I believe the problem had to do with filename extensions; as such, I
think the following three places constitute the actual fix for .
But he also cleaned up a few other little bits, which seems sensible and
ok since it's a pretty small patch.
LGTM, please push directly to staging
http://codereview.appspot.com/5563047/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
looks mostly good.
http://codereview.appspot.com/5520056/diff/7001/Documentation/notation/rhythms.itely
File Documentation/notation/rhythms.itely (right):
http://codereview.appspot.com/5520056/diff/7001/Documentation/notation/rhythms.itely#newcode1358
Documentation/notation/rhythms.itely:1358:
very funny.
http://codereview.appspot.com/5569050/diff/4001/Documentation/notation/changing-defaults.itely
File Documentation/notation/changing-defaults.itely (right):
http://codereview.appspot.com/5569050/diff/4001/Documentation/notation/changing-defaults.itely#newcode3721
LGTM
http://codereview.appspot.com/5564043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
I'm not certain if 2 days is a good number. Phil, after examining the
specific issues, would you say that 95% of real bug reports are handled
within 2 days? or should we make that 3 or 4 days instead?
http://codereview.appspot.com/5575047/
___
LGTM
http://codereview.appspot.com/5569045/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
On 2012/01/25 22:37:20, mail_philholmes.net wrote:
As a Brit, I would write it slightly less definitively. Please allow
a few
days.
I probably shouldn't have found that as funny as I did. :)
I still don't like that, though. As a user, when I report a bug, I want
to know that it's been
LGTM
http://codereview.appspot.com/5576053/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM, although I don't understand any of the pure/unpure stuff.
http://codereview.appspot.com/5569050/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM
http://codereview.appspot.com/5504100/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM
http://codereview.appspot.com/5520056/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
I'm not certain that these need to be inside @knownissues rather than
the main portion of the doc, but it's not worth quibbling.
Looks good enough to me.
http://codereview.appspot.com/5576063/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
LGTM, but I'd like David or Mike to give it the ok. If they do, I think
this can be pushed directly.
http://codereview.appspot.com/5623044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM*, please push immediately.
* or rather: I dislike the US spelling of canceled vs. the proper
British spelling cancelled, however our doc guide is quite clear: we
use US spelling. As such, the patch Looks Correct To Me, regardless of
my personal feelings on the matter. :)
Looks basically good, I just have some trivial quibbles. This should
vastly simplify debugging doc problems, though!
http://codereview.appspot.com/5645046/diff/1/Documentation/GNUmakefile
File Documentation/GNUmakefile (right):
http://codereview.appspot.com/5569045/diff/3001/input/regression/lilypond-book/GNUmakefile
File input/regression/lilypond-book/GNUmakefile (right):
http://codereview.appspot.com/5569045/diff/3001/input/regression/lilypond-book/GNUmakefile#newcode19
input/regression/lilypond-book/GNUmakefile:19:
LGTM, but I think it should go through a normal countdown.
http://codereview.appspot.com/5645046/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
I have mixed feelings about this patch. Given that adding osx 10.4 ppc
support is a two-line patch -- smaller than this one! -- it feels a bit
weird to move ahead with this. However, there is clearly no desire to
actually create that two-line patch, so I guess that removing support is
the thing
this codereview issue.
- Graham
http://codereview.appspot.com/5303063/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM
http://codereview.appspot.com/5650064/diff/4001/scripts/build/run-and-check.sh
File scripts/build/run-and-check.sh (right):
http://codereview.appspot.com/5650064/diff/4001/scripts/build/run-and-check.sh#newcode2
scripts/build/run-and-check.sh:2: eval $1 $2 21
I know this has already been
LGTM
http://codereview.appspot.com/5504092/diff/20001/scripts/auxiliar/lily-git.tcl
File scripts/auxiliar/lily-git.tcl (right):
http://codereview.appspot.com/5504092/diff/20001/scripts/auxiliar/lily-git.tcl#newcode248
scripts/auxiliar/lily-git.tcl:248: git remote add -t $originHead \
This
http://codereview.appspot.com/5669047/diff/1/Documentation/contributor/regressions.itexi
File Documentation/contributor/regressions.itexi (right):
http://codereview.appspot.com/5669047/diff/1/Documentation/contributor/regressions.itexi#newcode511
Documentation/contributor/regressions.itexi:511:
http://codereview.appspot.com/5665047/diff/1/Documentation/included/helpus.itexi
File Documentation/included/helpus.itexi (right):
http://codereview.appspot.com/5665047/diff/1/Documentation/included/helpus.itexi#newcode89
Documentation/included/helpus.itexi:89: @ignore
I'm ok with removing
http://codereview.appspot.com/5626052/diff/23001/mf/GNUmakefile
File mf/GNUmakefile (right):
http://codereview.appspot.com/5626052/diff/23001/mf/GNUmakefile#newcode231
mf/GNUmakefile:231: $(LILYPOND_BINARY) --verbose
$(top-src-dir)/ly/generate-font-integrals $@
This looks good, but not
http://codereview.appspot.com/5669047/diff/5001/Documentation/contributor/regressions.itexi
File Documentation/contributor/regressions.itexi (right):
http://codereview.appspot.com/5669047/diff/5001/Documentation/contributor/regressions.itexi#newcode512
This has been accepted; Pavel, please send the final patch to somebody
(not me) for pushing. Hopefully you have a mentor? Maybe Janek?
http://codereview.appspot.com/5651077/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
LGTM
http://codereview.appspot.com/5674101/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM
http://codereview.appspot.com/5494069/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM, please push to staging immediately.
http://codereview.appspot.com/5689058/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM. If David agrees, then I support pushing this to staging
immediately.
http://codereview.appspot.com/5688059/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM, please push directly to staging.
http://codereview.appspot.com/5698066/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
http://codereview.appspot.com/5696069/diff/1/Documentation/contributor/lsr-work.itexi
File Documentation/contributor/lsr-work.itexi (right):
http://codereview.appspot.com/5696069/diff/1/Documentation/contributor/lsr-work.itexi#newcode302
Documentation/contributor/lsr-work.itexi:302: @item
On
LGTM
http://codereview.appspot.com/5696073/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
On 2012/03/04 16:50:20, Julien Rioux wrote:
I think the scrips abc2ly and midi2ly are useful as stand-alone tools,
i.e.
outside of the build system, in which case users might still expect to
see some
progress messages. If that's the case then adding either a --quiet or
a
--verbose option
If you add a new section to the website without changing the relevant
part of Documentation/lilypond-texi2html.init, the colors will get
messed up. See line 1516.
http://codereview.appspot.com/5753059/diff/1/Documentation/web/community.itexi
File Documentation/web/community.itexi (right):
LGTM. Wait another few hours (until 21:00 or 22:00 UST), then push to
staging. If you're going to bed before then, go ahead and push to
staging earlier. If there's problems that aren't noticed before you
push it, we can fix the page later. Make sure it's in staging before
James runs patch
LGTM, please push directly to staging.
http://codereview.appspot.com/5783073/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
http://codereview.appspot.com/5754074/diff/3001/Documentation/notation/changing-defaults.itely
File Documentation/notation/changing-defaults.itely (right):
http://codereview.appspot.com/5754074/diff/3001/Documentation/notation/changing-defaults.itely#newcode2289
The only problem I can imagine is: what version of imagemagick is in
GUB, and what version of imagematick is required to handle svgs
properly?
http://codereview.appspot.com/5815043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
looks basically good, but two minor points.
Make that three minor points: please CC to -devel, not -devl.
http://codereview.appspot.com/5829043/diff/1/ly/articulate.ly
File ly/articulate.ly (right):
http://codereview.appspot.com/5829043/diff/1/ly/articulate.ly#newcode482
ly/articulate.ly:482:
ok. In the future, when updating a patch, please point git-cl at your
existing issue (which was orginally 2404, but I'm going to close 2404
and 2405 and leave 2406).
At a rough analogy, the code.google issue number is a pointer, while the
rietveld is a piece of memory. We don't care how often
images LGTM
http://codereview.appspot.com/5846043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM
http://codereview.appspot.com/5812043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM
http://codereview.appspot.com/5811043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
ok
http://codereview.appspot.com/5727055/diff/11002/scripts/abc2ly.py
File scripts/abc2ly.py (right):
http://codereview.appspot.com/5727055/diff/11002/scripts/abc2ly.py#newcode1400
scripts/abc2ly.py:1400: help=_ (suppress progress messages))
On 2012/03/12 19:43:38, Julien Rioux wrote:
http://codereview.appspot.com/5848056/diff/1/scripts/lilypond-book.py
File scripts/lilypond-book.py (right):
http://codereview.appspot.com/5848056/diff/1/scripts/lilypond-book.py#newcode104
scripts/lilypond-book.py:104: progress(_ ('%s (GNU LilyPond) %s' %
(ly.program_name,
looks basically good
http://codereview.appspot.com/5843060/diff/1/Documentation/usage/running.itely
File Documentation/usage/running.itely (right):
http://codereview.appspot.com/5843060/diff/1/Documentation/usage/running.itely#newcode56
Documentation/usage/running.itely:56: * Basic command
LGTM
http://codereview.appspot.com/5846075/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM
http://codereview.appspot.com/5843069/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
http://codereview.appspot.com/5870043/diff/1/python/auxiliar/postprocess_html.py
File python/auxiliar/postprocess_html.py (right):
http://codereview.appspot.com/5870043/diff/1/python/auxiliar/postprocess_html.py#newcode71
python/auxiliar/postprocess_html.py:71: browser_language_url =
http://codereview.appspot.com/5843069/diff/17/Documentation/common-macros.itexi
File Documentation/common-macros.itexi (right):
http://codereview.appspot.com/5843069/diff/17/Documentation/common-macros.itexi#newcode185
Documentation/common-macros.itexi:185:
This feels like a dangerous change. Obviously we don't /want/
bleed-through of parameters or memory from one file to the next, but I
would be shocked if we don't have any of that right now. I also have
vague memories of some problems with this in the past.
I guesstimate that not reloading init
I'm seeing old chunk mismatch for this patch. Could you try uploading
it again to a new rietveld issue?
http://codereview.appspot.com/5846075/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
On 2012/03/22 09:14:34, dak wrote:
Don't overestimate the issue. What is involved here is bleedthrough
from
init.ly, namely from one parser incarnation to the next. Any
bleedthrough in
scm is left unchanged.
ah, I misunderstood that part. Ok, in that case I feel much better
about the
LGTM.
Somebody might complain that the translations of misc/ should go in
their respective directories, but I think that's overkill, and I'd
really like to get this patch in. Removing old /web/ is finally within
reach! :)
http://codereview.appspot.com/5870043/
Awesome work as always. There's some unintended (and bad) changes to
GNUmakefile.in.
It's also missing the changelog and announcement for 2.14, but that can
get added in a different patch. The important thing is to get this
infrastructure pushed.
LGTM
http://codereview.appspot.com/5843060/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM, I think this could be pushed immediately?
http://codereview.appspot.com/5903046/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
The basic question is this: is this test helpful for programmers? At
first glance I'd say that it is, provided that you update the syntax as
David suggested.
IMO it takes a pretty silly example for something to *not* be a helpful
test case. Sure, having complicated scheme here might mean that
LGTM, please push immediately.
http://codereview.appspot.com/5900048/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM. You can send this to James for pushing whenever you want,
provided that it passes Patchy test-patches.
You might prefer to wait a day or two in case any scheme person has some
good ideas to make it better; partly to increase the patch quality, but
also for your own education about scheme.
Looks quite plausible; please push directly to staging.
http://codereview.appspot.com/5938048/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM; since it's just a snippet now, I support pushing it directly as
long as it compiles.
http://codereview.appspot.com/5882053/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM, please push directly to staging.
(patchy staging-merge is good enough; no need to wait for test-patches)
http://codereview.appspot.com/5956046/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
LGTM
http://codereview.appspot.com/5846075/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
could you split this into 2 (or more) separate patches?
- some of your comments are good, and could have been pushed a month
ago.
- the macro changes are highly problematic and probably won't be
accepted for at least a few months.
I never like seeing good changes postponed due to them being
The .latex file just says upload in progress. Could you add it to the
exceptions (like .scm and .ps) as discussed somewhere in the CG?
Assuming that the change to the .latex file is just like the .lytex
ones, please push the whole thing directly to staging.
(I'd still like the .latex extension
LGTM, please push immediately assuming it passes make doc.
http://codereview.appspot.com/5976056/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
sure, go ahead
http://codereview.appspot.com/5991068/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
for the next few weeks.
http://codereview.appspot.com/5975054/diff/1/lily/note-collision.cc#newcode577
lily/note-collision.cc:577: for_UP_and_DOWN (d) // please, make a
comment to
this loop (better than the above one...)
On 2012/04/01 05:00:25, Graham Percival wrote:
adding a comment to say please
LGTM, please push immediately.
http://codereview.appspot.com/6007048/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
wow, looks very nice and simple. I vote for pushing immediately, but
please wait for at least one other such vote (or a countdown).
http://codereview.appspot.com/6035053/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
LGTM; a few minor nitpicks.
http://codereview.appspot.com/6031053/diff/1/Documentation/contributor/quick-start.itexi
File Documentation/contributor/quick-start.itexi (left):
http://codereview.appspot.com/6031053/diff/1/Documentation/contributor/quick-start.itexi#oldcode17
LGTM, and I think we can skip the countdown for this one.
http://codereview.appspot.com/6062043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM and can be pushed right now.
But for my general ignorance: why not just hard-code 4? I suppose that
using the sizeof() trick makes it a bit safer in case somebody adds
something to ptrs, but is there any other reason?
http://codereview.appspot.com/6057044/
1 - 100 of 6136 matches
Mail list logo
