lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: add stencil-whiteout-outline function (issue 236480043 by address@hi


From: Paul Morris
Subject: Re: add stencil-whiteout-outline function (issue 236480043 by address@hidden)
Date: Mon, 8 Jun 2015 14:23:58 -0400

> On Jun 8, 2015, at 2:07 AM, address@hidden wrote:
> 
> On 2015/06/07 20:43:54, Keith wrote:
>> lgtm, if you can narrow the convert-ly rule a bit and test it (the
>> semi-automated 'patchy' testing doesn't use convert-ly)
> 
>> Also, I think you need to add
> 
>> Documentation/snippets/new/blanking-staff-lines-using-the--whiteout-command.ly
>> and let the makelsr script update the copy in /snippets/, but I'm
>> always
>> confused about that so just check the Contibutors’ Guide

I wondered about that.  The CG indicates that if you need to *manually* edit a 
snippet, then do it by copying the file to /snippets/new, run makelsr, etc.  

The "blanking-staff-lines-using-the—whiteout-command.ly 
<http://whiteout-command.ly/>" snippet didn’t need any *manual* changes after 
running “update-with-convert-ly.sh”, so that’s why I didn’t put it into /new/ 
(like I did with the other snippet that required a manual edit to the doc 
string).  

Is the CG correct on this?  CG 10.9.5 reads:

"Where the convert-ly rule is not able to automatically update snippets in 
Documentation/snippets/, those snippets must be manually updated. Those 
snippets should be copied to Documentation/snippets/new."


> Yes, do not edit the snippet directly in ../snippets/.. or add it
> directly to the snippets/.. directory.

I see, thanks, but what to do if running “update-with-convert-ly.sh” changes 
snippets in the /snippets/ directory for you?

Sounds like I should not 'git commit' those changed files in /snippets/, and 
then make the needed changes by copying the files to /snippets/new/.  Then 
maybe there’s some way to discard or revert the automatic changes to the files 
in the /snippets/ directory?

> If you are testing the patch yourself (even if you are not doing a full
> make doc) you will need to run the makelsr script before testing,
> however it is usually best practice to NOT apply the patch for review
> with the makelsr run, but generally run the makelsr after the patch has
> been pushed and push that separately. It makes it easier to revert or
> keep track of changes - some makelsr changes can produce a lot of tiny
> changes.

Ok.  Sounds like the CG would benefit from some updating on this point, as it 
currently simply says to run makelsr (in 10.9.5) before submitting for review 
(in 10.9.9).

Sounds like you are saying there’s a way to run makelsr to test a patch myself 
but then discard those changes so that what is reviewed doesn’t include the 
results of makelsr?  (I’m not sure how to do that…)  And then do a separate 
follow-up patch that has the makelsr changes.

Anyway, it sounds like I should just add the edited snippets to /snippets/new/ 
and not run makelsr as part of this patch/issue.

Thanks for the help sorting this out.  I think the next step for this issue is 
for me to create a new branch and redo all the changes on that branch with 
better commit messages and now that I have a better understanding of the 
convert-ly and snippet procedures.  Then upload that patch set for a 
(hopefully) final review.

-Paul



reply via email to

[Prev in Thread] Current Thread [Next in Thread]