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

Issue 5155045: LSR: Updated snippet for MMR Positions (1931) (Closed)

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

Description

LSR: Updated snippet for MMR Positions (1931) Removed statement that default MMR position was #0 (it is actually #2) Added some examples of the overrides and clarified some comments including adding new example of more than two voices

Patch Set 1 #

Total comments: 9

Patch Set 2 : Removed translations texidocs #

Patch Set 3 : Trevor's Edit requests #

Patch Set 4 : patch based on trevors last comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+59 lines, -0 lines) Patch
A Documentation/snippets/new/positioning-multi-measure-rests.ly View 1 2 1 chunk +59 lines, -0 lines 0 comments Download

Messages

Total messages: 10
Trevor Daniels
A couple of nitpicks, otherwise LGTM (but untested). (I'll abandon my patch - yours is ...
7 years, 8 months ago (2011-10-01 21:19:49 UTC) #1
Graham Percival (old account)
http://codereview.appspot.com/5155045/diff/1/Documentation/snippets/new/positioning-multi-measure-rests.ly File Documentation/snippets/new/positioning-multi-measure-rests.ly (right): http://codereview.appspot.com/5155045/diff/1/Documentation/snippets/new/positioning-multi-measure-rests.ly#newcode1 Documentation/snippets/new/positioning-multi-measure-rests.ly:1: \version "2.14.0" On 2011/10/01 21:19:50, Trevor Daniels wrote: > ...
7 years, 8 months ago (2011-10-02 10:14:50 UTC) #2
Graham Percival (old account)
wait a moment, why are you adding a snippet to git at all? Just fix ...
7 years, 8 months ago (2011-10-02 10:18:12 UTC) #3
Trevor Daniels
This is to illustrate a change introduced in e75f38b1a9adaf7752ced683fff0e6ec01bd8a13, which appeared first in 2.13.63 and ...
7 years, 8 months ago (2011-10-02 15:08:09 UTC) #4
Graham Percival (old account)
ok, fair enough. By now LSR *should* be running 2.14, but since it's not I ...
7 years, 8 months ago (2011-10-02 15:23:59 UTC) #5
pkx166h
Nitpicks done. I'm still not sure of the process here Do I need to delete ...
7 years, 8 months ago (2011-10-06 19:55:24 UTC) #6
t.daniels_treda.co.uk
James, you wrote Thursday, October 06, 2011 8:55 PM > Do I need to delete ...
7 years, 8 months ago (2011-10-06 21:47:19 UTC) #7
Graham Percival (old account)
LGTM
7 years, 8 months ago (2011-10-07 21:29:58 UTC) #8
pkx166h
Had problems uploading new patch to Rietveld after I ran makelsr.py - there are 60+ ...
7 years, 8 months ago (2011-10-08 06:12:36 UTC) #9
Graham Percival (old account)
7 years, 8 months ago (2011-10-10 13:05:27 UTC) #10
yes, fine, LGTM, push it.
Sign in to reply to this message.

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