Hi Mark, I think it's an improvement. I've made specific comments inline.
Thanks! Carl http://codereview.appspot.com/1056041/diff/11001/12001 File Documentation/learning/common-notation.itely (right): http://codereview.appspot.com/1056041/diff/11001/12001#newcode313 Documentation/learning/common-notation.itely:313: c4-^ c-+ c-- c-| | c4-> c-. c2-_ | Why 2 bar checks? http://codereview.appspot.com/1056041/diff/11001/12001#newcode927 Documentation/learning/common-notation.itely:927: Girls and boys come | out to play, Do you like putting bar checks in Lyrics? I've never done that. Maybe I should. http://codereview.appspot.com/1056041/diff/11001/12001#newcode1339 Documentation/learning/common-notation.itely:1339: c,4 d, e, f, | g,4 a, b, c | d4 e f g | a4 b c' d' | Why move this from multiple lines to a single line? This example seems to me to be one where the pedagogy isn't tied to a single line, so perhaps it's a good one to do the "one measure per line" rule. http://codereview.appspot.com/1056041/diff/11001/12002 File Documentation/learning/fundamental.itely (right): http://codereview.appspot.com/1056041/diff/11001/12002#newcode1516 Documentation/learning/fundamental.itely:1516: dum dum | dum dum | I think you have made an interesting choice here to reorder the content. This is an argument I have with myself, and I come up with different answers on different projects. Should I group everything by musical section (as you've done here), or by content type (as was done previously). I *think* my personal preference in by content type, but I suppose that could change next week... http://codereview.appspot.com/1056041/diff/11001/12002#newcode1585 Documentation/learning/fundamental.itely:1585: cis4 cis2. Why not a bar check here? http://codereview.appspot.com/1056041/diff/11001/12002#newcode1586 Documentation/learning/fundamental.itely:1586: g4 Why an incomplete bar? http://codereview.appspot.com/1056041/diff/11001/12002#newcode2237 Documentation/learning/fundamental.itely:2237: \new Voice \relative c' { I think the GDP code standards say that it should be \new Voice { \relative c' { http://codereview.appspot.com/1056041/diff/11001/12002#newcode2258 Documentation/learning/fundamental.itely:2258: \new Voice \relative c' { I think the proper fix for this is to add the { after \new Voice http://codereview.appspot.com/1056041/diff/11001/12002#newcode2280 Documentation/learning/fundamental.itely:2280: \new Staff << Aha -- I finally have figured out why I see so may examples of \new Staff << >>. This has caused problems later on. I recommend that we change all of these \new Staff << >> to \new Staff { } http://codereview.appspot.com/1056041/diff/11001/12002#newcode3185 Documentation/learning/fundamental.itely:3185: @c Skylining handles this correctly without padText How about changing the padText command to a textFont command that does an override on font size? http://codereview.appspot.com/1056041/diff/11001/12003 File Documentation/learning/tutorial.itely (right): http://codereview.appspot.com/1056041/diff/11001/12003#newcode501 Documentation/learning/tutorial.itely:501: thumb is to indent code blocks with either a tab or two spaces: If our standard is two spaces, we should say two spaces, instead of a tab or two spaces. http://codereview.appspot.com/1056041/diff/11001/12003#newcode679 Documentation/learning/tutorial.itely:679: c4-\markup { I don't prefer a standard that says we need to break up this markup. The markup fits comfortably on one line, And I think it's better keeping it as an integrated whole. If it didn't fit on one line, then I could see using this formatting. http://codereview.appspot.com/1056041/show _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
