|
|
Created:
14 years, 6 months ago by Valentin Villenave Modified:
14 years, 5 months ago Reviewers:
Graham Percival, haberg-1, carl.d.sorensen, Trevor Daniels, Graham Percival (old account) CC:
lilypond-devel_gnu.org Visibility:
Public. |
DescriptionRemove arabic.ly from common note name languages.
Patch Set 1 #
Total comments: 2
Patch Set 2 : Rephrase NR pitches, and add stubs to glossary #
Total comments: 14
Patch Set 3 : Minor corrections; thanks Trevor! #
Total comments: 7
MessagesTotal messages: 17
Greetings everybody, This commit could make it easier for me to document upcoming changes to the note names selection syntax. Please have a look, and feel free to push if it's okay.
Sign in to reply to this message.
LBetterTM, except for excessive TODOs http://codereview.appspot.com/2755041/diff/1/Documentation/notation/world.itely File Documentation/notation/world.itely (right): http://codereview.appspot.com/2755041/diff/1/Documentation/notation/world.ite... Documentation/notation/world.itely:62: @c TODO: add Glossary entry for maqam? -vv Don't need three TODOs for essentially the same thing http://codereview.appspot.com/2755041/diff/1/Documentation/notation/world.ite... Documentation/notation/world.itely:420: @c TODO: add Glossary entry for makam? -vv Don't need three TODOs for essentially the same thing
Sign in to reply to this message.
LGTM. I agree with Trevor's comments. Carl
Sign in to reply to this message.
On 2010/10/26 21:36:03, Carl wrote: > LGTM. > > I agree with Trevor's comments. OK, I have merged all three TODOs in a single one in world.itely. Since I have both of your approvals, I am pushing the patch now. Thanks! Valentin.
Sign in to reply to this message.
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 review doc patches on this topic. Are you *trying* to sabotage the 2.14 release process?! You know why we didn't have 2.14 out in 2009? It's because we had a whole bunch of commits which didn't receive proper attention, regtests were broken and nobody noticed, and generally we accumulated a huge "bug debt" that we're now almost finished "paying off". Calm the bloody mao down. Doc patches _do_ get approved. James Lowe has been steadily cleaning up broken documentation; his patches sometimes take a week and 3-4 versions (notwithstanding when I push one by accident), but he gets stuff done. Pushing a half-baked patch after only waiting 83 minutes for comments is *not* being fair to him and all the hard work he's been doing. I'm sorry I was asleep when you sent the patch and didn't comment earlier, but we cannot rely on developers being awake and working on lilypond all the time. For your next patch, please wait at least 24 hours before pushing, regardless of how positive the reviews are.
Sign in to reply to this message.
On Wed, Oct 27, 2010 at 8:58 AM, <percival.music.ca@gmail.com> wrote: > I have reverted this commit. For some reason, I wouldn't expect otherwise. :-) > Calm the bloody mao down. Doc patches _do_ get approved. James Lowe > has been steadily cleaning up broken documentation; his patches > sometimes take a week and 3-4 versions (notwithstanding when I push one > by accident), but he gets stuff done. Pushing a half-baked patch after > only waiting 83 minutes for comments is *not* being fair to him and all > the hard work he's been doing. If it's half-baked, then please do comment on it. If it's not, then you're just reverting things for the sake of reverting things. > I'm sorry I was asleep when you sent the patch and didn't comment > earlier, but we cannot rely on developers being awake and working on > lilypond all the time. For your next patch, please wait at least 24 > hours before pushing, regardless of how positive the reviews are. Whilst I understand the need to make it a matter of principles, if you don't mind me asking: have you *looked* at the patch? It's a three-sentences modification, for crying out loud! On a subject (removing arabic.ly) that we already discussed at length, and where we all agreed (AFAICR). Hadn't I ever heard anything from you on this subject, then of course I wouldn't have dreamt of pushing this patch without your blessing. Now, you said you had some suggestions; I'm looking forward to hearing about these. Cheers, Valentin.
Sign in to reply to this message.
On Wed, Oct 27, 2010 at 10:28 AM, Valentin Villenave <v.villenave@gmail.com> wrote: > If it's half-baked, then please do comment on it. I would have rather commented in the codereview interface, but oh well. - pitches.itely, line 600 in new version: was there supposed to be a newline here? I'm not certain why you started a new sentence on a new line, so I thought it was worth checking this. (I don't think that it _should_ be a new paragraph, but it's not clear what your intention was) - same place, but more generally: I'm not certain quite what these paragraphs are getting at (perhaps seeing it in a bit more context would have helped), but I think they could be improved. - world.itely, line 20: I *really* don't like the comment. If it's a TODO, then make sure you add a "TODO" string for ease of greppiness. That said, I don't like seeing TODOs; I'd rather have a new issue in the tracker. That said*2, wtf don't you just add the music glossary entries yourself? If you don't know what to write in the Glossary, you can still add the entry as a stub. And then add a doc item for "fill in stubs: makam, maqam, makamlarasdqrs". Remember that new doc writers find @nodes and @ref{}s confusing, so if old-timers prepare the general layout of the text files, it can save newbies literally hours. I've added stubs a few times for new doc contributors. - More generally, I'd rather see more clarity about languages vs. music styles. It's not really clear to me (as a quick+ineffectual reader) why Arabic isn't just one more language. - finally, yes, I'm wanting the patch to be *better*-quality than the original material. And I don't make any apology for that. > Whilst I understand the need to make it a matter of principles, if you > don't mind me asking: have you *looked* at the patch? Yes. > On a subject > (removing arabic.ly) that we already discussed at length, and where we > all agreed (AFAICR). Yes, we did. > Hadn't I ever heard anything from you on this > subject, then of course I wouldn't have dreamt of pushing this patch > without your blessing. Umm, didn't you hear from me here: http://lists.gnu.org/archive/html/lilypond-devel/2010-10/msg00401.html For clarity: 1. Yes, I agree with the general aim of the new language format, and treating arabic.ly separately from those. 2. I think the current direction of the code is fantastic, and I'm really really glad to see you working on it. 3. However, I don't want to rush in. In particular, I want to review any doc changes. 4. In particular*2, I want to review the FINAL version of any doc patch. After you've made any changes that other people asked. 5. In particular*3, I'm not going to drop everything and look at a new patch as soon as it goes online. I want at least a 24-hour period to look at the patch. In case #4 sounds like I'm being arrogant and disregarding other developers: no, not at all. Basically, whenever you have a "final draft", I want it to be on codereview, and to get nothing but "LGTM" or "+1" from people, for at least 24 hours. Once that's done, go ahead and push. If you get ANY comments other than "LGTM/+1" -- even if it's just "hey, there's a typo over here; you should replace "teh" with "the" -- then I want to see an updated patch on codereview. Which waits for another 24-hour period. Cheers, - Graham
Sign in to reply to this message.
On 2010/10/27 10:27:30, graham_percival-music.ca wrote: > - same place, but more generally: I'm not certain quite what these > paragraphs are getting at (perhaps seeing it in a bit more context > would have helped), but I think they could be improved. I think it's best if we treat non-Western stuff as "notations and tunings" rather than just "note names". Here's a new patch set, please have a look. > - world.itely, line 20: I *really* don't like the comment. If it's a > TODO, then make sure you add a "TODO" string for ease of greppiness. > That said, I don't like seeing TODOs; I'd rather have a new issue in > the tracker. That said*2, wtf don't you just add the music glossary > entries yourself? If you don't know what to write in the Glossary, > you can still add the entry as a stub. And then add a doc item for > "fill in stubs: makam, maqam, makamlarasdqrs". Remember that new doc > writers find @nodes and @ref{}s confusing, so if old-timers prepare > the general layout of the text files, it can save newbies literally > hours. I've added stubs a few times for new doc contributors. Indeed. However, it's pretty hard to draw the line between what we add and what we disregard. What I'd suggest (see patch) is to open a new chapter within the MG. I think - we have enough material to afford that, - these terms are specialized enough to allow for really elaborate MG entries, - it avoids cluttering the "Classical/Western" MG with totally unrelated terms and concepts, - if anything, having a new @chapter to fill *might* encourage users to help us expand it. > - More generally, I'd rather see more clarity about languages vs. > music styles. It's not really clear to me (as a quick+ineffectual > reader) why Arabic isn't just one more language. Hence my proposal of not mentioning "note names" anymore in the non-Western section's title. > - finally, yes, I'm wanting the patch to be *better*-quality than the > original material. And I don't make any apology for that. Well, Trevor did mention that it "looked better" :) > In case #4 sounds like I'm being arrogant and disregarding other > developers: no, not at all. Basically, whenever you have a "final > draft", I want it to be on codereview, and to get nothing but "LGTM" > or "+1" from people, for at least 24 hours. Once that's done, go > ahead and push. I don't blame you for wanting to review stuff and have the final say. (Well, let me rephrase that: Wanting to review stuff and have the final say, is not what I blame you for. :-) OK, now all I have to do is find something to do for the next 24 hours... :) Cheers.
Sign in to reply to this message.
On 27 Oct 2010, at 14:53, v.villenave@gmail.com wrote: > I think it's best if we treat non-Western stuff as "notations and > tunings" rather than just "note names". Here's a new patch set, > please > have a look. As it now stands in the manual, it looks out of context to me. So it should be changed, I think: The situation in world music is the same as in CPP: there is a set of pitches that strictly speaking can refer to different tunings. Then people may choose different ways to name those notes. For example in gamelan music, some may use English and others Indonesian names. So strictly speaking, I think there should be headers: CPP, Arabic, Persian, Turkish, etc., and under each, there might be one or more files producing those. Also, I prefer CPP (Common Practice Period) to "Western", as it becomes complicated to explain Western music traditions outside CPP.
Sign in to reply to this message.
A few editorial suggestions ... some apply to other similar instances, which I've not marked. http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/pitch... File Documentation/notation/pitches.itely (left): http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/pitch... Documentation/notation/pitches.itely:579: Many non-Western musics (and some Western folk and Other types of non-Western music http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/pitch... Documentation/notation/pitches.itely:580: traditional musics) employ alternative or extended tuning music http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/pitch... File Documentation/notation/pitches.itely (right): http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/pitch... Documentation/notation/pitches.itely:590: systems. Some of these musics, like Arabic music, may Some of these, like http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/pitch... Documentation/notation/pitches.itely:593: implicitely determined by context. Other types of music, implicitly ... Others, however http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/vocal... File Documentation/notation/vocal.itely (right): http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/vocal... Documentation/notation/vocal.itely:632: Ky -- ri -- e __ Quite correct! But it shouldn't be part of this patch. http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/world... File Documentation/notation/world.itely (right): http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/world... Documentation/notation/world.itely:93: Music Glossary should come before Notation Reference http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/world... Documentation/notation/world.itely:261: @notation{rast}, @rglos{ } ??
Sign in to reply to this message.
Thanks Trevor! New patch set. http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/pitch... File Documentation/notation/pitches.itely (left): http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/pitch... Documentation/notation/pitches.itely:579: Many non-Western musics (and some Western folk and On 2010/10/27 17:41:18, Trevor Daniels wrote: > Other types of non-Western music Er... no. Actually, These types music are not opposite to the former, but even do include Arabic music! Please read again and tell me what I could do to make it more clear... http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/pitch... Documentation/notation/pitches.itely:580: traditional musics) employ alternative or extended tuning On 2010/10/27 17:41:18, Trevor Daniels wrote: > music Done. http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/pitch... File Documentation/notation/pitches.itely (right): http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/pitch... Documentation/notation/pitches.itely:590: systems. Some of these musics, like Arabic music, may On 2010/10/27 17:41:18, Trevor Daniels wrote: > Some of these, like Done. http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/pitch... Documentation/notation/pitches.itely:593: implicitely determined by context. Other types of music, On 2010/10/27 17:41:18, Trevor Daniels wrote: > implicitly ... Others, however Done. http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/vocal... File Documentation/notation/vocal.itely (right): http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/vocal... Documentation/notation/vocal.itely:632: Ky -- ri -- e __ On 2010/10/27 17:41:18, Trevor Daniels wrote: > Quite correct! But it shouldn't be part of this patch. Oops, my git branches got messed up. http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/world... File Documentation/notation/world.itely (right): http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/world... Documentation/notation/world.itely:93: On 2010/10/27 17:41:18, Trevor Daniels wrote: > Music Glossary should come before Notation Reference Are you sure? I thought that what should come first were links to the same manual (i.e. @ref links). http://codereview.appspot.com/2755041/diff/10001/Documentation/notation/world... Documentation/notation/world.itely:261: @notation{rast}, On 2010/10/27 17:41:18, Trevor Daniels wrote: > @rglos{ } ?? Indeed.
Sign in to reply to this message.
On Wed, Oct 27, 2010 at 9:11 PM, <v.villenave@gmail.com> wrote: > Are you sure? I thought that what should come first were links to the > same manual (i.e. @ref links). Oh, I wasn't looking at the right section: I was looking at http://lilypond.org/doc/v2.13/Documentation/contributor/syntax-survey#cross-r... instead of http://lilypond.org/doc/v2.13/Documentation/contributor/section-organization Sorry! Valentin.
Sign in to reply to this message.
On 27 Oct 2010, at 19:41, tdanielsmusic@googlemail.com wrote: > A few editorial suggestions ... some apply to other similar instances, > which I've not marked. The description of Turkish music is rather cursory: there are several descriptions. See for example Ozan Yarman, "A Comparative Evaluation of Pitch Notations in Turkish Makam Music". The LilyPond text refers, I take it, to the Arel-Ezgi-Uzdilek (AEU) system. It does divide the major second M into 9 parts, but the minor second m is divided into 4; so it is 2*m + 5*M = 2*4 + 5*9 = 53 equal temperament. The file makam.ly, I recall, departs from E12 dividing the major second into 9 parts. So it should be in E108. So I avoid using the terms "whole" and "half" tones, because the half tone is not in general a half of the whole tone. :-) Also, when listing the note names as c, d, e, ..., then that refers to the octave numbering of major scales which I think was introduced by Helmholtz, possibly influenced by Romantic period centralizing CPP major/minor harmony. At the Maqam World site <http://www.maqamworld.com/>, they number the octaves according to the minor scale. I do not know why, but perhaps they have never started using the Helmholtz system. So perhaps the notes should be named a, b, c, ... in this context. Just mentioning it. When LilyPond expand being capable of handling more music outside CPP, there might be more such details pooping up.
Sign in to reply to this message.
Nearly there :) http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/pitch... File Documentation/notation/pitches.itely (right): http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/pitch... Documentation/notation/pitches.itely:588: Many non-Western musics (and some Western folk and Many types of non-Western music (and ... [It's "musics" I don't like] http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/pitch... Documentation/notation/pitches.itely:652: @rglos{makamlar}. Music Glossary is placed before Notation Reference http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/world... File Documentation/notation/world.itely (right): http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/world... Documentation/notation/world.itely:92: @rglos{maqam}. Music Glossary comes before Notation Reference
Sign in to reply to this message.
comments. http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/pitch... File Documentation/notation/pitches.itely (left): http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/pitch... Documentation/notation/pitches.itely:447: use default (Nederlands) note names, the @code{@bs{}include} Incidently, does your language change mean that this limitation is no longer true? That would be awesome. http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/pitch... File Documentation/notation/pitches.itely (right): http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/pitch... Documentation/notation/pitches.itely:41: * Non-Western notations and tunings:: WTM is "notations" ? http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/pitch... Documentation/notation/pitches.itely:444: @code{@w{@bs{}include "english.ly"}} to the input file. If you're going to change this, then you might as well change it properly. ------- For example, to use English note names, use: @lilypond[quote,verbatim] \language "english" \relative c'' { c4 cs cf css } @end lilypond ----- I thought you were only doing the arabic stuff here, but oh well. http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/pitch... Documentation/notation/pitches.itely:570: @unnumberedsubsubsec Non-Western notations and tunings Why have an @unnumberedsubsubsec here at all? I suggest a single sentence in the previous bit, saying something like "Some types of non-Western music use alternate pitches; these are discussed in @ref{World music}." Then we can safely isolate all that stuff to the relevant Specialist 2.x section. Your job as a programmer and doc writer is over -- as long as it's clear where this weird stuff should be discussed, you're fine. Let somebody who cares about the alternate notations write the descriptions. You're not expected to care about everything that your work touches. And even if you *do* care about it, I'd rather see any music theory descriptions as a separate patch, anyway. Focus on the _specific_ topic at hand and get the patch approved+pushed. There's always time for more patches later on.
Sign in to reply to this message.
On 28 Oct 2010, at 01:01, tdanielsmusic@googlemail.com wrote: > http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/pitch... > File Documentation/notation/pitches.itely (right): > > http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/pitch... > Documentation/notation/pitches.itely:588: Many non-Western musics (and > some Western folk and > Many types of non-Western music (and ... > > [It's "musics" I don't like] You might use "types of music".
Sign in to reply to this message.
On 28 Oct 2010, at 06:03, percival.music.ca@gmail.com wrote: > http://codereview.appspot.com/2755041/diff/22001/Documentation/notation/pitch... > Documentation/notation/pitches.itely:41: * Non-Western notations and > tunings:: > WTM is "notations" ? You might use "notation and tuning systems" in that context. A tuning is more specific: once one has fixed the system to use, it may have several different tunings. In LilyPond, one must currently choose a tuning along with the tuning system.
Sign in to reply to this message.
|