lilypond-devel
[Top][All Lists]

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

 From: percival . music . ca Subject: Re: Doc: LM: Reformat ly code. (issue1056041) Date: Wed, 05 May 2010 18:14:18 +0000

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 explicit durations
-- that are getting delayed / possibly lost in the shuffle.

I don't know what the answer is... I mean, if I had a patch that only
did the everybody-agreed-upon things (like the explicit durations), I'd
happily push that, and then we could continue debating the contentious
stuff.  OTOH, if I were you, I wouldn't want to put this patch, create a
new patch with only the durations (and whatever else we agree on), push
that, then rebase my original patch to match the new git thing.  OTOH*2,
maybe you're getting sick of having long discussions about patches, and
would like to get *something* pushed.

http://codereview.appspot.com/1056041/diff/11001/12001
File Documentation/learning/common-notation.itely (right):

http://codereview.appspot.com/1056041/diff/11001/12001#newcode409
Documentation/learning/common-notation.itely:409: a2_\markup {
What was wrong with the 1 durations?  Why change to 2 ?

http://codereview.appspot.com/1056041/diff/11001/12001#newcode492
Documentation/learning/common-notation.itely:492: \partial 8 f8 | c2 d |
I'd rather see a newline inside here.  We could debate about whether the
f8 should be on the same line as the \partial 8 or on a separate line,
but the "c2 d" definitely belongs on a new line.

http://codereview.appspot.com/1056041/diff/11001/12001#newcode515
Documentation/learning/common-notation.itely:515: \times 2/3 { d4 a8 } |
The barline check here does nothing to help us understand the example.

http://codereview.appspot.com/1056041/diff/11001/12001#newcode974
Documentation/learning/common-notation.itely:974: @c TODO: If we keep
the bar checks in the lyrics above, we should
As you know, I'm vehemently opposed to adding TODOs to "finished" docs.
I'm ok with this right now since we're just discussing the patch, but it
had better not be pushed with this still in here.

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' |
On 2010/05/05 10:57:06, Carl wrote:

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.


Agreed.  This example was just fine to begin with, other than adding a 4
to the beginning of each line.

http://codereview.appspot.com/1056041/show