Code review - Issue 560030044: Remove deprecated context propertieshttps://codereview.appspot.com/2021-01-06T19:57:21+00:00rietveld
Message from unknown
2020-05-09T16:17:13+00:00Valentin Villenaveurn:md5:87f6a69c5d511963115e400384bf71f0
Message from lemzwerg@googlemail.com
2020-05-09T18:25:03+00:00lemzwergurn:md5:f5d5f5760d132d920a74e59e2316d5c3
LGTM. This issue also adds some scheme code and a conversion rule – they are different commits, right?
https://codereview.appspot.com/560030044/diff/557800043/scm/define-context-properties.scm
File scm/define-context-properties.scm (right):
https://codereview.appspot.com/560030044/diff/557800043/scm/define-context-properties.scm#newcode609
scm/define-context-properties.scm:609: merge rests when this is set to true.")
While you are at it, please fix indentation (i.e., no indentation for the continuation line).
Message from thomasmorley65@gmail.com
2020-05-09T20:17:09+00:00thomasmorley651urn:md5:e93f8e2aef89e574f87da81ef8f8f9f2
Not sure about chordNameExceptionsFull and chordNameExceptionsPartial.
I thought our deprecating policy would be to wait for 1 or even 2 stable versions before deleting from source.
Well, the snippet demonstrates a user-level reimplementation, but this makes little sense to me. Also, banter-style is gone as well, afaik.
Message from v.villenave@gmail.com
2020-05-10T08:04:02+00:00Valentin Villenaveurn:md5:b77625704a152793c1dfd1329ee6b52b
On 2020/05/09 20:17:09, thomasmorley651 wrote:
> Not sure about chordNameExceptionsFull and chordNameExceptionsPartial.
> I thought our deprecating policy would be to wait for 1 or even 2 stable
> versions before deleting from source.
Actually, all that was deprecated _long_ ago, much longer than 1 or 2 stable releases:
https://lists.gnu.org/archive/html/lilypond-user/2008-08/msg00389.html
I just made that clearer (and even added a convert-rule for banter-style) :
https://git.savannah.gnu.org/cgit/lilypond.git/commit/?id=78225bc1b386e12dc1d03a5d2c7a017c0a52a22d
V.
Message from v.villenave@gmail.com
2020-05-10T08:08:14+00:00Valentin Villenaveurn:md5:d764da8961cfa480f2cb4ee4848a4c16
On 2020/05/09 18:25:03, lemzwerg wrote:
> https://codereview.appspot.com/560030044/diff/557800043/scm/define-context-properties.scm#newcode609
> scm/define-context-properties.scm:609: merge rests when this is set to true.")
> While you are at it, please fix indentation (i.e., no indentation for the
> continuation line).
Acknowledged.
> LGTM. This issue also adds some scheme code and a conversion rule – they are
> different commits, right?
Should they really be? The scheme code (largely inspired by Harm BTW :-) is only needed for the snippet (which will get updated through next makelsr anyway).
As for define-context-properties.scm, it’s used in the doc so I thought it’d make sense to have that in the same commit that the @funindex cleanup.
V.
Message from roypjohnson76@gmail.com
2021-01-06T19:57:21+00:00roypjohnson76urn:md5:4e13dee55d6ae36ee342ecbea93ea045