Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(560)

Issue 3705042: Doc: More NR and LM additions based on user emails (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 6 months ago by pkx166h
Modified:
8 years, 5 months ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Doc: More NR and LM additions based on user emails http://lists.gnu.org/archive/html/lilypond-user/2010-12/msg00067.html Repeats.itely (NR) Added additional information to @warning about not placing anything outside of the braced groups within an \alternative block. Original comments for this patch to this file were in http://codereview.appspot.com/3581041/patch/1/2. I have made some adjustments to the original suggestion incorporating comments but also adding bar checks to the examples in this specific section to help illustrate what this statememt means rather than having to create an @example that doesn't work in the doc. http://lists.gnu.org/archive/html/lilypond-user/2010-08/msg00372.html Fundamental.itely (LM) Added @knownissue about nothing being printed if the notehead engraver is turned off, even though the expected behaviour is that beams and stems would still be printed.

Patch Set 1 #

Total comments: 16

Patch Set 2 : Doc: More NR and LM additions based on user emails - with corrections #

Total comments: 2

Patch Set 3 : More Amendments #

Total comments: 1

Patch Set 4 : Final amendments (based on TD's comments) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -12 lines) Patch
M Documentation/learning/fundamental.itely View 1 1 chunk +6 lines, -1 line 0 comments Download
M Documentation/notation/repeats.itely View 1 2 3 4 chunks +15 lines, -11 lines 0 comments Download

Messages

Total messages: 15
pkx166h
Hello, This is another patch following on from http://codereview.appspot.com/3581041/ James
8 years, 6 months ago (2010-12-16 18:28:21 UTC) #1
Graham Percival (old account)
http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundamental.itely File Documentation/learning/fundamental.itely (right): http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundamental.itely#newcode2114 Documentation/learning/fundamental.itely:2114: objects to note heads, if you remove the @code{Note_heads_engraver} ...
8 years, 6 months ago (2010-12-17 14:05:27 UTC) #2
Carl
Thanks, James! Carl http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundamental.itely File Documentation/learning/fundamental.itely (right): http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundamental.itely#newcode2114 Documentation/learning/fundamental.itely:2114: objects to note heads, if you ...
8 years, 6 months ago (2010-12-17 14:21:03 UTC) #3
pkx166h
http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.itely File Documentation/notation/repeats.itely (right): http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.itely#newcode150 Documentation/notation/repeats.itely:150: @rprogram{An extra staff appears}. Do not place bar checks ...
8 years, 6 months ago (2010-12-17 19:07:51 UTC) #4
Carl
Fine to push, but one last try on the barcheck warning. Thanks, Carl http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.itely File ...
8 years, 6 months ago (2010-12-17 19:13:45 UTC) #5
Trevor Daniels
Like Carl, I'm happy to approve this as it is, but I'd prefer to see ...
8 years, 6 months ago (2010-12-17 21:02:21 UTC) #6
pkx166h
Thanks for the input. Corrections are done. http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundamental.itely File Documentation/learning/fundamental.itely (right): http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundamental.itely#newcode2114 Documentation/learning/fundamental.itely:2114: objects to ...
8 years, 6 months ago (2010-12-19 11:27:33 UTC) #7
Carl
I like it, and think it's ready for pushing. Just for purposes of clarification, let ...
8 years, 6 months ago (2010-12-19 14:45:14 UTC) #8
Graham Percival
On Sun, Dec 19, 2010 at 02:45:14PM +0000, Carl.D.Sorensen@gmail.com wrote: > So if somebody has ...
8 years, 6 months ago (2010-12-20 18:36:31 UTC) #9
pkx166h
Hello, Is this OK to give someone a patch to push? James
8 years, 6 months ago (2010-12-21 13:30:38 UTC) #10
Trevor Daniels
Looks fine to me, but then you've used the wording I suggested, so it would, ...
8 years, 6 months ago (2010-12-21 15:21:29 UTC) #11
Carl
On 2010/12/21 15:21:29, Trevor Daniels wrote: > Looks fine to me, but then you've used ...
8 years, 6 months ago (2010-12-21 15:29:28 UTC) #12
Trevor Daniels
Hi James I think a further change is needed to position the warning in repeats ...
8 years, 6 months ago (2010-12-22 11:09:52 UTC) #13
pkx166h
Done. However I think that the two 'warning' boxes will look a bit awkward, but ...
8 years, 6 months ago (2010-12-22 12:37:41 UTC) #14
Trevor Daniels
8 years, 6 months ago (2010-12-22 23:43:19 UTC) #15
On 2010/12/22 12:37:41, pkx166h wrote:

> I think that the two 'warning' boxes will look a bit awkward

Maybe, but the placement makes the warning much more relevant.  Let's leave for
a couple of days, and if there are no further comments send me a patch.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b