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

Issue 4839061: NR Context Layout Order rewrite (5.1.7) - tracker 1812 (Closed)

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

Description

Doc: NR remove 5.1.7 Tracker issue 1812 Removed 5.1.7 as it was felt that this was already explaind in 5.1.6 the use of \accepts and \denies is now only required when creating new contexts.

Patch Set 1 #

Patch Set 2 : Need some guidance by those that understand this more than I #

Total comments: 8

Patch Set 3 : With Trevor D's suggestions. Thanks Trevor. #

Total comments: 1

Patch Set 4 : Added @lilypond example #

Total comments: 14
Unified diffs Side-by-side diffs Delta from patch set Stats (+53 lines, -88 lines) Patch
M Documentation/notation/changing-defaults.itely View 1 2 3 4 chunks +53 lines, -88 lines 14 comments Download

Messages

Total messages: 9
Trevor Daniels
James My suggestion was to remove the *material* in 5.1.7 and replace it with material ...
7 years, 11 months ago (2011-08-10 22:32:13 UTC) #1
pkx166h
On 2011/08/10 22:32:13, Trevor Daniels wrote: > James > My suggestion was to remove the ...
7 years, 9 months ago (2011-09-24 20:49:23 UTC) #2
Trevor Daniels
James Looks pretty good. I've made a couple of suggested additions. Could you please make ...
7 years, 9 months ago (2011-09-24 22:17:30 UTC) #3
pkx166h
New Patch set loaded with Trevor's suggestions. James http://codereview.appspot.com/4839061/diff/3001/Documentation/notation/changing-defaults.itely File Documentation/notation/changing-defaults.itely (right): http://codereview.appspot.com/4839061/diff/3001/Documentation/notation/changing-defaults.itely#newcode935 Documentation/notation/changing-defaults.itely:935: @cindex ...
7 years, 9 months ago (2011-09-25 12:07:17 UTC) #4
Graham Percival (old account)
http://codereview.appspot.com/4839061/diff/7002/Documentation/notation/changing-defaults.itely File Documentation/notation/changing-defaults.itely (right): http://codereview.appspot.com/4839061/diff/7002/Documentation/notation/changing-defaults.itely#newcode961 Documentation/notation/changing-defaults.itely:961: @example could this be a @lilypond instead?
7 years, 9 months ago (2011-09-27 04:35:56 UTC) #5
pkx166h
On 2011/09/27 04:35:56, Graham Percival wrote: > http://codereview.appspot.com/4839061/diff/7002/Documentation/notation/changing-defaults.itely > File Documentation/notation/changing-defaults.itely (right): > > http://codereview.appspot.com/4839061/diff/7002/Documentation/notation/changing-defaults.itely#newcode961 ...
7 years, 9 months ago (2011-10-01 15:11:45 UTC) #6
Trevor Daniels
A few comments, but otherwise LGTM. If you agree the changes and it compiles please ...
7 years, 9 months ago (2011-10-01 20:27:17 UTC) #7
Graham Percival (old account)
I agree with everything Trevor said, including pushing after you've changed it and it compiles. ...
7 years, 9 months ago (2011-10-02 06:21:09 UTC) #8
pkx166h
7 years, 9 months ago (2011-10-06 20:50:14 UTC) #9
Thanks Trevor and Graham. I also had to change an @ref in vocal.itely because
the node name changed.

Checked it all compiled.

commit	32c862967d559ab512cd96e3321a2c8dabe724ca

http://codereview.appspot.com/4839061/diff/11001/Documentation/notation/chang...
File Documentation/notation/changing-defaults.itely (right):

http://codereview.appspot.com/4839061/diff/11001/Documentation/notation/chang...
Documentation/notation/changing-defaults.itely:950: contain it.  This can give
rise to unexpected new staves or scores.
On 2011/10/01 20:27:17, Trevor Daniels wrote:
> I think this might be better placed at the end of this section.

Done.

http://codereview.appspot.com/4839061/diff/11001/Documentation/notation/chang...
Documentation/notation/changing-defaults.itely:961: @lilypond[quote]
On 2011/10/01 20:27:17, Trevor Daniels wrote:
> Hmm.  I think we do need verbatim here too to make it clear what is being
> demonstrated.

Done.

http://codereview.appspot.com/4839061/diff/11001/Documentation/notation/chang...
Documentation/notation/changing-defaults.itely:961: @lilypond[quote]
On 2011/10/01 20:27:17, Trevor Daniels wrote:
> Hmm.  I think we do need verbatim here too to make it clear what is being
> demonstrated.

Done.

http://codereview.appspot.com/4839061/diff/11001/Documentation/notation/chang...
Documentation/notation/changing-defaults.itely:963: \new Staff { c' d' e' f'
On 2011/10/01 20:27:17, Trevor Daniels wrote:
> newline and indent

Done.

http://codereview.appspot.com/4839061/diff/11001/Documentation/notation/chang...
Documentation/notation/changing-defaults.itely:964: \chords { d1:m7 b1:min7.5- }
On 2011/10/01 20:27:17, Trevor Daniels wrote:
> indent

Done.

http://codereview.appspot.com/4839061/diff/11001/Documentation/notation/chang...
Documentation/notation/changing-defaults.itely:971: \new Staff { c' d' e' f'
On 2011/10/01 20:27:17, Trevor Daniels wrote:
> newline and indent

Done.

http://codereview.appspot.com/4839061/diff/11001/Documentation/notation/chang...
Documentation/notation/changing-defaults.itely:972: \chords { d1:m7 b1:min7.5- }
On 2011/10/01 20:27:17, Trevor Daniels wrote:
> indent

Done.
Sign in to reply to this message.

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