Re: Doc: LM: Reformat ly code. (issue1056041)

I've only reviewed the first file. I don't think that reviewing more would do good; we're still debating simple things like "should there be a barline check at the end of every single line". Seeing a patch doesn't help with that debate. At the same time, there's good changes -- like adding expl

Re: Doc: LM: Reformat ly code. (issue1056041)

Looking much closer now. http://codereview.appspot.com/1056041/diff/24001/25001 File Documentation/learning/common-notation.itely (right): http://codereview.appspot.com/1056041/diff/24001/25001#newcode147 Documentation/learning/common-notation.itely:147: a1 | This example doesn't need a bar che

Re: Doc: Reorganize music functions material. (issue1031044)

The patch looks ok, but I'm getting some weird build errors... quite possibly the same thing Werner noticed (wherein lilypond-book borks if it has two identical snippets). My main goal today was to check Patrick's build system changes, so I'll have to leave it with this vague warning. If you can

Re: Doc: LM: Reformat ly code. (issue1056041)

Looks (mostly) good to me, other than the below 2 items. http://codereview.appspot.com/1056041/diff/33001/27003 File Documentation/learning/fundamental.itely (right): http://codereview.appspot.com/1056041/diff/33001/27003#newcode1339 Documentation/learning/fundamental.itely:1339: @c TODO: This

Re: Doc: NR: Using \partial with \repeat. (issue1136044)

Looks fine to me. http://codereview.appspot.com/1136044/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Doc: NR: Reformat ly code. (issue1242044)

Well, there's 40 minutes of my 60 minutes of lilypond work for today done. What's the first thing we tell new doc contributors? Send small patches. I think this applies here. http://codereview.appspot.com/1242044/diff/2001/3001 File Documentation/notation/changing-defaults.itely (right): htt

Re: Doc: NR: Reformat ly code. (issue1242044)

I only had time to examine changing-defaults. I'll review the other files tonight. http://codereview.appspot.com/1242044/diff/30002/34001 File Documentation/notation/changing-defaults.itely (right): http://codereview.appspot.com/1242044/diff/30002/34001#newcode357 Documentation/notation/changi

Re: Doc: NR: Reformat ly code. (issue1242044)

I managed to trudge through another 4 or 5 files. Look, this is ridiculous. I'm officially rejecting this patch because it's **WAY** too long for anybody to look at. Please break this into separate patches, where each patch is 1500 lines or less. If a single file has more than 1500 diff-lines,

LilyLib: error and warning as functions, program_name and _version as variables (issue1240042)

Looks good to me; it builds the docs from scratch. Please push. http://codereview.appspot.com/1240042/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Doc: Contributor's: Update Regression tests (issue1545043)

Looks mostly good. http://codereview.appspot.com/1545043/diff/1/2 File Documentation/contributor/programming-work.itexi (right): http://codereview.appspot.com/1545043/diff/1/2#newcode1305 Documentation/contributor/programming-work.itexi:1305: Yes, -jX CPU_COUNT=X would be useful, but I'd rather

Re: Lilypond-book: Factor out the formatting from lilypond-book into separate classes (issue1543042)

Looks good to me; it compiles cleanly from scratch, and all doc formats look normal. (I couldn't check images inside info, since I don't know how to display those) http://codereview.appspot.com/1543042/show ___ lilypond-devel mailing list lilypond-deve

Add regression tests for lilypond-book (issue1556043)

The code looks ok, but I couldn't check it due to the docbook issue (below). http://codereview.appspot.com/1556043/diff/1/45 File make/lilypond-book-rules.make (right): http://codereview.appspot.com/1556043/diff/1/45#newcode48 make/lilypond-book-rules.make:48: cd $(outdir) && dblatex $(notdir $

Re: Doc: Contributor's: Update Regression tests (issue1545043)

Looks good to me! http://codereview.appspot.com/1545043/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Add regression tests for lilypond-book (issue1556043)

sorry, no joy: cd ./out-www && @PDFLATEX@ suffix-latex.tex /bin/sh: @PDFLATEX@: not found make[4]: *** [out-www/suffix-latex.pdf] Error 127 gperc...@gperciva-desktop:/main/src/build-lilypond$ grep PDFLATEX * config.make:PDFLATEX = @PDFLATEX@ http://codereview.appspot.com/1556043/show __

Re: Add regression tests for lilypond-book (issue1556043)

Hmm, I've just built it from scratch without problems using -j3 CPU_COUNT=3. Odd. I don't know what happened last time. Ok, let's go ahead and merge this. http://codereview.appspot.com/1556043/show ___ lilypond-devel mailing list lilypond-devel@gnu.

Re: feta-accordion.mf, convertrules.py, musicxml2ly.py: sanitize accordion symbol names (issue1908041)

On 2010/07/29 13:01:47, c_sorensen_byu.edu wrote: Right now you have put these conversions into a separate rule. Add them to the 2.13.29 rule. Then I think you have to do make to get the rules into convert-ly, but I'm not sure on that. You might need to do something like make -C scripts

Re: Fix 1084. (issue1871058)

Looks ok here; I just did a build from scratch with it applied. The only potential issue is whether it works with the regtest comparison, but we can't test that until I make another release. Please push. :) http://codereview.appspot.com/1871058/show ___

T1041 - Add descriptions for \bookOutputSuffix and \bookOutputName to documents (NR). (issue2004041)

Looks mostly good. http://codereview.appspot.com/2004041/diff/1/2 File Documentation/notation/input.itely (right): http://codereview.appspot.com/2004041/diff/1/2#newcode110 Documentation/notation/input.itely:110: @file{fandangoforelephants.pdf} (or @file{fandangoforelephants.png}, or .eps, or .

Build: fix 775 configure should detect doc reqs. (issue2019041)

Reviewers: , Message: Ok, hearing no complaints... and since this doesn't add any build requirements (it just checks if you have the existing reqs, instead of letting you attempt to make doc and then die halfway through), I pushed it. Description: Doc: CG: update requirements for doc build. Bu

Re: Doc: CG: rewrite Lilybuntu docs, move to 1.3. (issue2093041)

Reviewers: carl.d.sorensen_gmail.com, Message: Thanks, pushed. I also pushed an extra sentence to the CG with your tip about Rietveld. Description: Doc: CG: rewrite Lilybuntu docs, move to 1.3. Ping, does this thing send emails to -devel or not? Please review this at http://codereview.appspot

Regtests: add missing descriptions (half fix 1206) (issue2103043)

Reviewers: , Message: The song-* stuff will be done later (without review, since nobody particularly cares about the festival stuff, and those regtest names are pretty explanatory anyway). Description: Regtests: add missing descriptions (half fix 1206) This adds descriptions to all regtests ot

Build: un-nodify markup-*-commands.tely (issue2140041)

Reviewers: , Message: This isn't a miracle of clean scheme, but I don't think it's completely horrible, gets the job done, and makes the doc-editor's lives easier. Description: Build: un-nodify markup-*-commands.tely Please review this at http://codereview.appspot.com/2140041/ Affected files:

Re: Build: un-nodify markup-*-commands.tely (issue2140041)

On 2010/09/03 17:41:37, Carl wrote: scm/documentation-generate.scm:56: ;; magic number to remove the initial part Can we have something in the comment that says what would have to change in order to change the magic number? Thanks for the comment; done. I should have done this math to begi

Clarify infinity_f preprocessor definition. (issue2099043)

Reviewers: , Message: I have another tiny clarity rewrite for code. Description: Clarify infinity_f preprocessor definition. The old code confuses various style programs, including our own fixcc.py (it produces un-compilable code!). In particular: - const Real infinity_f = #ifdef - whi

Re: Clarify infinity_f preprocessor definition. (issue2099043)

On 2010/09/06 14:23:36, Carl wrote: LGTM. Thanks, pushed. Anything we can do rationally to get automatic style fixers to work on our code should be done, IMO. About 2.5 hours of work so far; not bad. I think I'm about halfway. Cheers, - Graham http://codereview.appspot.com/2099043/ _

Style: don't break compiling freetype-errors.cc (issue2144047)

Reviewers: , Message: I'd appreciate it if a python guru / regex person could take a quick glance at this. These changes will stop fixcc.py rendering lily/freetype-errors.cc uncompilable, but this is the most complicated regex I've ever attempted (which doesn't say much; I've never needed to us

Re: Style: don't break compiling freetype-errors.cc (issue2144047)

On 2010/09/11 07:02:55, perpeduumimmobile wrote: Wouldn't it be easier to just say - if a line belongs to a preprocessor statement (i.e. it either starts with "#" or each of the lines before end with "\", and the first line of those starts with "#") then leave it alone (dont

Re: [Patch] Absolute dynamics as postfix text (issue2220041)

Initial comments, not a complete review. BTW, these files just say "upload in progress". Could you re-upload the patch, maybe after a few fixes? scm/define-markup-commands.scm scm/font.scm scm/ly-syntax-constructors.scm http://codereview.appspot.com/2220041/diff/1/Doc

Re: Doc: NR 1.5.2: Clarify voice order; \shiftOn etc. (issue2226045)

http://codereview.appspot.com/2226045/diff/3001/Documentation/notation/simultaneous.itely File Documentation/notation/simultaneous.itely (right): http://codereview.appspot.com/2226045/diff/3001/Documentation/notation/simultaneous.itely#newcode392 Documentation/notation/simultaneous.itely:392: @e

Re: Doc: NR 1.5.2: Clarify voice order; \shiftOn etc. (issue2226045)

On 2010/09/23 15:22:52, t.daniels_treda.co.uk wrote: markpolesky wrote Thursday, September 23, 2010 3:51 PM > as I think more about visual-impairment accessibility, I > feel that the @example is immediately readable without the > code clutter. I'll remove it if you feel strongly. As this i

Re: Doc: NR 1: Use @co...@var{...}} for variables. (issue2217046)

LGTM, although I didn't check that it still compiles. http://codereview.appspot.com/2217046/diff/1/Documentation/notation/pitches.itely File Documentation/notation/pitches.itely (right): http://codereview.appspot.com/2217046/diff/1/Documentation/notation/pitches.itely#newcode142 Documentation/n

Re: Doc: NR 1: Use @co...@var{...}} for variables. (issue2217046)

LGTM, go ahead an push. Technically, the doc policy (or at least custom) is to have a blank newline before the @end itemize / @end enumerate, but I don't know if that has any effect on anything. I just always do it that way because previous material did it that way, and I was never curious enoug

Doc: CG: move reitveld into CG 2, rewrite. (issue2348041)

Reviewers: , Message: Fix for 1202. Description: Doc: CG: move reitveld into CG 2, rewrite. Please review this at http://codereview.appspot.com/2348041/ Affected files: M Documentation/contributor/programming-work.itexi M Documentation/contributor/source-code.itexi _

Re: Style: don't break compiling freetype-errors.cc (issue2144047)

I just checked a second-order diff (i.e. produce a diff by running an unpatched fixcc, then produce a diff by running the patched fixcc, then compare those diffs), and the only two changes are: 1) don't change */ ; into: */; (this seems like a very good change) 2) keep lily/freetype-e

Re: Optimizations for pure-height approximations. (issue1817045)

Has this patch been pushed, or definitely rejected, or is it waiting for when Joe has some spare time (probably 2011 :) ? http://codereview.appspot.com/1817045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinf

Re: Adds ly:define-event-class (issue1867050)

As far as I can see, Mike has resolved all the known problems with this patch. Are there any other problems, or should we accept it? http://codereview.appspot.com/1867050/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mai

Re: Doc: NR 4.1.2: Reorganize vertical dimensions. (issue2316042)

On 2010/10/03 04:50:12, joeneeman wrote: On Sat, Oct 2, 2010 at 8:08 PM, wrote: > I think we should document what we have, and make enhancement requests > for what we would *like* to have. I don't think the exercise of "write > the documents for what we

Re: Doc: NR 4.1.2: Reorganize vertical dimensions. (issue2316042)

LGTM, although I have no knowledge of the spacing code. http://codereview.appspot.com/2316042/diff/1/Documentation/notation/spacing.itely File Documentation/notation/spacing.itely (right): http://codereview.appspot.com/2316042/diff/1/Documentation/notation/spacing.itely#newcode198 Documentation

Style: disable removal of "gratuitous" blocks. (issue2380041)

Reviewers: , Message: Here's two patches (confusingly merged together by git cl :( that complete the "make fixcc.py produce compilable files" task. Description: Style: disable removal of "gratuitous" blocks. This rule removed any {} which were around a single statment, but this removal broke a

Rename vertical spacing dimensions. (issue2303044)

With regards to convert-ly, it would be nice to run it on the translations. But as long as the doc build still works, don't manually change anything in the translations. I'll be writing up policies for convert-ly later this evening or tomorrow. For the regtests, just go ahead and rename them.

convert: properly escape some single-backslashes. (issue2401042)

Reviewers: , Message: I noticed a few weird error messages while testing convert-ly on the docs. Description: convert: properly escape some single-backslashes. Please review this at http://codereview.appspot.com/2401042/ Affected files: M python/convertrules.py Index: python/convertrules.p

Re: convert: properly escape some single-backslashes. (issue2401042)

On 2010/10/11 18:18:37, dak wrote: python/convertrules.py:2992: _ ("\\RemoveEmpty*StaffContext -> \*Staff \\RemoveEmptyStaves")) Any particular reason that \*Staff does not get the backslash doubled as well? The reason was that I didn't know what \* did -- I'm guessing it has a special meaning

Re: convert: properly escape some single-backslashes. (issue2401042)

On 2010/10/12 06:23:56, dak wrote: mailto:percival.music...@gmail.com writes: Is that not a documentation string rather than a regexp? And the top level meaning is a string meaning, anyway, not a regexp one? Yes, sorry. I now see that all these changes are in the documentation strings; the

Build: another hack for translations (fix 1323). (issue2520041)

Reviewers: , Description: Build: another hack for translations (fix 1323). Please review this at http://codereview.appspot.com/2520041/ Affected files: M python/auxiliar/postprocess_html.py Index: python/auxiliar/postprocess_html.py diff --git a/python/auxiliar/postprocess_html.py b/pytho

Re: vertical spacing: Rename dimensions. (issue2505041)

LGTM, and compiles cleanly from scratch. http://codereview.appspot.com/2505041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Build: another hack for translations (fix 1323). (issue2520041)

On 2010/10/17 16:52:03, John Mandereau wrote: python/auxiliar/postprocess_html.py:357: extra_depth = '../' '../' is is not needed for translations, as output web pages are at the same depth as the pages in English, and may cause errors in case server configuration doesn't redirect /.. to /. I

Re: Build: another hack for translations (fix 1323). (issue2520041)

Here's another version. Note that the "extra depth" thing is now added to the *non*-translated version. I have no clue why. I've spent 3 hours bashing against the build process, and that's enough for me. CG 5.3 Debugging website and docs locally has what I know about trying to solve this stuf

convert-ly: option to only modify changed files. (issue2642041)

Reviewers: , Message: Pretty simple, but useful. Description: convert-ly: option to only modify changed files. This useful for the regtests -- with the extra -d flag, we will only change the \version string if the actual contents of the file has changed. This lets us see which tests were modif

Re: convert: properly escape some single-backslashes. (issue2401042)

On 2010/10/20 14:06:45, Carl wrote: LGTM. Carl thanks pushed. Closing this issue. http://codereview.appspot.com/2401042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: convert-ly: option to only modify changed files. (issue2642041)

On 2010/10/21 08:09:23, Graham Percival wrote: Pretty simple, but useful. argh, I meant to add: I'm not overly confident about the -d letter, nor the --diff-version-update name. Better suggestions are welcome. http://codereview.appspot.com/2642041/ ___

Re: convert-ly: option to only modify changed files. (issue2642041)

New patch: it still updates the version for a stable release. running convert-ly -d -eon: \version "2.11.0" { c'4 } produces: \version "2.12.0" { c'4 } which is exactly what we want for the regtests. http://codereview.appspot.com/2642041/ ___

Re: doc additions power chords (issue2686041)

Looks mostly fine. http://codereview.appspot.com/2686041/diff/1/Documentation/notation/fretted-strings.itely File Documentation/notation/fretted-strings.itely (right): http://codereview.appspot.com/2686041/diff/1/Documentation/notation/fretted-strings.itely#newcode1416 Documentation/notation/fr

Re: doc additions power chords (issue2686041)

LGTM, go ahead and push. http://codereview.appspot.com/2686041/diff/5001/Documentation/notation/fretted-strings.itely File Documentation/notation/fretted-strings.itely (right): http://codereview.appspot.com/2686041/diff/5001/Documentation/notation/fretted-strings.itely#newcode1422 Documentation

Re: convert-ly: option to only modify changed files. (issue2642041)

On 2010/10/24 10:50:52, Carl wrote: LGTM. Thanks, pushed. http://codereview.appspot.com/2642041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Reorganize language files and add a new \language command. (issue2699041)

It's not _the_ charm, but it's certainly much more charming. Calm down, take your time, and make it beautiful. I'm not going to start the 2.14 release candidate without this patch, so there's no rush. Expect to do 1-3 more candidate patches before it's finally accepted. http://codereview.apps

Doc: CG: clarify convert-ly usage policy. (issue2687043)

Reviewers: , Message: Policy highlights: - convert-ly will be the responsibility of the programmer, not the doc+translation people. (however, translators are still on the hook for the current round of updates, as per issue 1358) - easy copy&paste lines to run convert-ly Description: Doc: CG: c

Re: Doc: CG: clarify convert-ly usage policy. (issue2687043)

On 2010/10/25 12:45:38, Carl wrote: I'll make a script and put it in my ~/bin that will do the convert-ly. Do we want to put it in scripts/auxiliar? Hmm. Initially I was just expecting people to copy&paste the relevant commands... but a shell script is even easier (and more reliable!). The

Re: Doc: CG: clarify convert-ly usage policy. (issue2687043)

On 2010/10/25 13:04:03, Graham Percival wrote: ok, ignore this patch; I'll make an improved one tomorrow. New patch uploaded; it's much easier, and the doc section is much smaller. http://codereview.appspot.com/2687043/ ___ lilypond-devel mailing l

Re: Remove arabic.ly from common note name languages. (issue2755041)

On 2010/10/26 22:13:26, Valentin Villenave wrote: Since I have both of your approvals, I am pushing the patch now. Thanks! I have reverted this commit. You put something up for review for 83 minutes, you don't wait for approval from the Documentation Editor -- and you *know* that I want to rev

Re: Reorganize language files and add a new \language command. (issue2699041)

Looks mostly good. http://codereview.appspot.com/2699041/diff/24001/Documentation/changes.tely File Documentation/changes.tely (right): http://codereview.appspot.com/2699041/diff/24001/Documentation/changes.tely#newcode74 Documentation/changes.tely:74: be used in safe mode). The old syntax is

Re: Doc: CG: clarify convert-ly usage policy. (issue2687043)

On 2010/10/26 17:19:51, Carl wrote: L Great TM. this has now been pushed. http://codereview.appspot.com/2687043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Reorganize language files and add a new \language command. (issue2699041)

LGTM http://codereview.appspot.com/2699041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Remove arabic.ly from common note name languages. (issue2755041)

comments. http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/pitches.itely File Documentation/notation/pitches.itely (left): http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/pitches.itely#oldcode447 Documentation/notation/pitches.itely:447: use defaul

Re: Allow predefined diagrams regardless of note names language. (issue2791041)

LGTM. (for clarity: none of the comments made so far will force a second draft, so if nobody has a complaint in the next 5 hours, you're good to push it) http://codereview.appspot.com/2791041/diff/1/ly/predefined-guitar-fretboards.ly File ly/predefined-guitar-fretboards.ly (right): http://coder

Re: Doc: move non-Western languages to world.itely (issue2735044)

Generally ok, but I don't know why you're changing the translations before the English docs are done. http://codereview.appspot.com/2735044/diff/1/Documentation/notation/pitches.itely File Documentation/notation/pitches.itely (right): http://codereview.appspot.com/2735044/diff/1/Documentation/n

Re: Doc: move non-Western languages to world.itely (issue2735044)

LGTM. My nitpick doesn't require a new draft version; if there are no other complaints, go ahead and push in 23 hours. I still don't like seeing TODOs, and in a few months we'll be going through and removing all TODOs from the code... but since these TODOs weren't yours, I can't blame you for ke

Doc: Removed [fragment] from @lilypond examples NR (issue2810041)

Reviewers: , Message: Docs look ok, but I'm not 100% certain about the changes to chords, or whether Trevor wants us changing vocal.itely. (this is a git-cl-merge of James Lowe's initial doc patch, and a few fixes I applied to that one; I don't think it's worth separating this into two separate

Re: Doc: Removed [fragment] from @lilypond examples NR (issue2810041)

On 2010/10/31 13:54:01, Carl wrote: I think the relative should only be included with figures: 1. When there are notes, and 2. When we want the notes in relative mode. Ok, I've removed all [relative=?] unless there's a combination of notes with \chordmode or \figures. Will upload once the

Re: Doc: NR: Move "Modifying alists" from 4.1.2 to 5.3. (issue2767043)

Looks ok as far as "cut&paste" goes, but I think it could use more work as a piece of our documentation. http://codereview.appspot.com/2767043/diff/1/Documentation/notation/changing-defaults.itely File Documentation/notation/changing-defaults.itely (right): http://codereview.appspot.com/2767043

Re: Doc: improve 2.10 World music (issue2732043)

http://codereview.appspot.com/2732043/diff/3001/Documentation/music-glossary.tely File Documentation/music-glossary.tely (right): http://codereview.appspot.com/2732043/diff/3001/Documentation/music-glossary.tely#newcode8765 Documentation/music-glossary.tely:8765: @node Non-Western terms A-Z Some

Re: Doc: NR: Move "Modifying alists" from 4.1.2 to 5.3. (issue2767043)

On 2010/11/02 11:58:20, Carl wrote: Maybe the solution is to show the modification of nested-alist properties *other* than spacing alists. I like that idea. Showing one or more of these in order to demonstrate the various methods might be more useful than using the spacing alists which ar

Re: Doc: NR: Move "Modifying alists" from 4.1.2 to 5.3. (issue2767043)

On 2010/11/02 18:18:12, Mark Polesky wrote: On 2010/11/02 11:58:20, Carl wrote: > Maybe the solution is to show the modification of > nested-alist properties *other* than spacing alists. > > [...] > > Showing one or more of these in order to demonstrate the > various methods might be more useful

Re: Doc: Removed [fragment] from @lilypond examples NR (issue2810041)

On 2010/11/01 18:35:14, Trevor Daniels wrote: I'm happy with the changes to vocal.itely, although ragged-right could also be removed in many (maybe all) of the @lilyponds. Aye, [ragged-right] should be removed from (almost?) all @lilypond. But this patch is too big already. Thanks, pushed.

Re: Doc: NR 4.4.1: Rewrite. (issue2642043)

http://codereview.appspot.com/2642043/diff/14001/Documentation/notation/spacing.itely File Documentation/notation/spacing.itely (right): http://codereview.appspot.com/2642043/diff/14001/Documentation/notation/spacing.itely#newcode256 Documentation/notation/spacing.itely:256: a system is the midd

Re: Doc: CG -- Add statement on lilypond-hackers list (issue2933041)

Looks fine. http://codereview.appspot.com/2933041/diff/1/Documentation/contributor/administration.itexi File Documentation/contributor/administration.itexi (right): http://codereview.appspot.com/2933041/diff/1/Documentation/contributor/administration.itexi#newcode169 Documentation/contributor/a

Re: Doc: CG -- Add statement on lilypond-hackers list (issue2933041)

sure, push it. http://codereview.appspot.com/2933041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Changes to Default spacing settings (issue3061041)

oops, we were missing the CC to lilypond-devel. The patch looks generally ok to me, but I'm not up to speed on spacing stuff. Keith: why are you removing the #'stretchability = 5 ? What's the default value if you remove it? James: I suggest that the next version of the patch uses the same ind

Doc: translations: manually update syntax (1358) (issue3092041)

Reviewers: , Message: Translation Meister: here's a fix for 1358. ok to push? I don't know why this issue has been hanging around for 3 week... I mean, editing .ly code and @funindex entries isn't precisely difficult! Description: Doc: translations: manually update syntax (1358) Please review

Re: Update spacing for Keith Ohara's suggested defaults (issue3099041)

Looks good to me, thanks Carl! I'll make .39 once this is applied, but let's wait 24 hours before pushing to give people in all time zones a chance to comment. http://codereview.appspot.com/3099041/ ___ lilypond-devel mailing list lilypond-devel@gnu.o

Re: Corrections and additions to NR 4.1.2, "Page formatting". (issue1710046)

On 2010/11/14 06:59:41, Mark Polesky wrote: I don't want to be a spoilsport, but I have a patch of my own that I'm pretty sure takes care of everything involved here (and more): http://codereview.appspot.com/2758042/ I have absolutely no problem with that. I just wanted some clarity about the

Re: Doc: translations: manually update syntax (1358) (issue3092041)

On 2010/11/14 07:13:08, pacovila wrote: Graham: here are my comments. I will not close the issue until all other languages are revised, but for German it is done. Thanks I'll close this patch request, at least. http://codereview.appspot.com/3092041/diff/1/Documentation/de/notation/fretted-

Re: Fix 1382 (issue3100041)

On 2010/11/14 00:49:38, Carl wrote: Here's a fix for Issue 1382. It sets the staff position to zero if staff space is zero, which is a consistent outcome -- all the staff lines are in the same position so zero works. Looks good, and the regtest comparison looks fine. Please push. - Graha

Re: Doc: NR 4.1: Reorganize, clarify details. (issue2758042)

Looks ok. http://codereview.appspot.com/2758042/diff/23001/Documentation/notation/input.itely File Documentation/notation/input.itely (right): http://codereview.appspot.com/2758042/diff/23001/Documentation/notation/input.itely#newcode759 Documentation/notation/input.itely:759: @snippets Err, th

Checks for git user details (issue3182041)

Reviewers: , Message: Patchset 1 contains the actual improvement to lily-git.tcl. It works on my machine, and I'll push it in 24 hours if nobody complains. Patchset 2 adds indentation from emacs. The commit message isn't showing up, so here it is: This is the only tcl file in our repo. It d

Re: Doc: NR 4.1: Reorganize, clarify details. (issue2758042)

On 2010/11/18 09:18:27, Mark Polesky wrote: However, I'd like to point out that I tried to organize paper-defaults-init.ly a little bit, and I need to know if I recklessly broke anything there by changing the order of the declarations. Can someone confirm that it's fine? Have you done a regtes

Re: Remove head- and foot-separation. (issue3199041)

On 2010/11/18 09:12:23, Mark Polesky wrote: Is that precisely how the conversion should be made? Can someone verify this? Should I push this patch or wait for a convert-ly rule? I'm getting a bit confused with all the renamings, reorganizations, etc., and I can't be the only one. Could we sl

Re: [Patch] Fix #1333: beamed multi-note acciaccaturas (issue3169041)

I can't view scm/music-functions.scm ; please upload again. http://codereview.appspot.com/3169041/diff/3001/ly/grace-init.ly File ly/grace-init.ly (left): http://codereview.appspot.com/3169041/diff/3001/ly/grace-init.ly#oldcode4 ly/grace-init.ly:4: startGraceMusic = { Why are these being remov

Re: Checks for git user details (issue3182041)

On 2010/11/18 01:15:51, Carl wrote: LGTM. Thanks, pushed. http://codereview.appspot.com/3182041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

manual convert-ly: minimum-Y-extent (1298+1299). (issue3212041)

Reviewers: , Message: Patch from Keith; long commit message explaining stuff. Description: manual convert-ly: minimum-Y-extent (1298+1299). Patch by Keith O'Hara. cary.ly used Y-extent to increase space for staves, use markup-system-spacing instead. orchestra.ly used Y-extent to created a cus

Re: Remove head- and foot-separation. (issue3199041)

On 2010/11/19 23:23:20, Mark Polesky wrote: Okay, this small patch is finished. Graham, would you like me to wait to push this for any reason? The patch looks ok to me. I agree with the 2.13.40 convert-ly rule, and the NOT_SMART-ness. At the moment, I can't think of any reason why somebody w

Re: manual convert-ly: minimum-Y-extent (1298+1299). (issue3212041)

On 2010/11/19 07:41:46, Patrick McCarty wrote: One question about the commit message: it mentions "engravers-one-by-one.ly", but there are no changes to this file in your patch. Should there be changes? This has been removed from the commit message. Otherwise, looks good to me. Thanks, p

Re: Doc: NR 4: Minor edits. (issue3406041)

On 2010/12/02 08:34:52, Trevor Daniels wrote: LGTM; just a few nitpicks I agree with Trevor; I noticed (almost) everything he did, and didn't notice anything else. http://codereview.appspot.com/3406041/ ___ lilypond-devel mailing list lilypond-devel

Re: Doc: NR 4: Minor edits. (issue3406041)

LTGM http://codereview.appspot.com/3406041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Doc: CG: clarify lily-git.tcl and git-cl. (issue3396042)

Reviewers: , Message: Please review. Description: Doc: CG: clarify lily-git.tcl and git-cl. Please review this at http://codereview.appspot.com/3396042/ Affected files: M Documentation/contributor/introduction.itexi M Documentation/contributor/source-code.itexi _

Re: Doc: CG: clarify lily-git.tcl and git-cl. (issue3396042)

On 2010/12/06 23:12:09, Valentin Villenave wrote: Hi Graham, LGTM as well. Just a few comments: Thanks, pushed. Documentation/contributor/source-code.itexi:143: @quotation Is there a rule for the use of @quotation? (I noticed there's another one in source-code.itexi.) If so, we probably sho

Re: Doc: Various to LM and NR from user email threads (issue3581041)

Thanks for this! I've slightly modified the git-cl instructions, to include "make the CC list lilypond-devel". That's part of the git-cl config stage, so you don't need to remember it for every individual patch you upload. http://codereview.appspot.com/3581041/diff/1/Documentation/learning/f

harmonics and slides (issue3590041)

Looks basically good. http://codereview.appspot.com/3590041/diff/1/Documentation/notation/fretted-strings.itely File Documentation/notation/fretted-strings.itely (right): http://codereview.appspot.com/3590041/diff/1/Documentation/notation/fretted-strings.itely#newcode383 Documentation/notation/

Re: Add /chordGlissando to music functions (issue3530042)

Could it go in a separate optional-include \include "guitar.ly" file? I'd sign off with no qualms if it were optional. http://codereview.appspot.com/3530042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinf

Re: Don't make paper columns if skipTypesetting = ##t. (issue3598041)

LGTM; regtest comparison shows nothing unexpected, and the docs build from scratch. Please push. http://codereview.appspot.com/3598041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fix bookpart identifier crash with page-marker. (issue3585042)

LGTM; regtest comparison shows nothing unexpected, and the docs build from scratch. Please push. http://codereview.appspot.com/3585042/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel

  1   2   3   4   5   >