Re: Doc: NR - 2.10.2 - Arabic Music - improved example (issue 572600048 by address@hidden)

[pkx116h]

Hello,

On 29/01/2020 22:47, address@hidden wrote:
> https://codereview.appspot.com/572600048/diff/548660043/Documentation/snippets/new/non-traditional-key-signatures.ly
> File Documentation/snippets/new/non-traditional-key-signatures.ly
> (right):
>
> https://codereview.appspot.com/572600048/diff/548660043/Documentation/snippets/new/non-traditional-key-signatures.ly#newcode5
> Documentation/snippets/new/non-traditional-key-signatures.ly:5:
> version-specific, non-traditional, world-music"
> Breaking this line breaks makelsr.py .  How did this get through?
>
> I see that it was silently fixed in
>
> commit c511518a4e7669ac7263f8a28565debefe25a358
> Author: James Lowe <address@hidden>
> Date:   Mon Apr 15 21:03:57 2019 +0100
>
>      makelsr run for previous commit
>      
>      Changes made based on commit
>      Doc: NR - 2.10.2 - Arabic Music - improved example
>      03f78e3e7bda039ea08744d3736bfa4a39dea2a4
>
> diff --git
> a/Documentation/snippets/new/non-traditional-key-signatures.ly
> b/Documentation/snippets/new/non-traditional-key-signatures.ly
> index f5967009bd..f2a16ca841 100644
> --- a/Documentation/snippets/new/non-traditional-key-signatures.ly
> +++ b/Documentation/snippets/new/non-traditional-key-signatures.ly
> @@ -1,8 +1,7 @@
>   \version "2.19.21"
>   
>   \header {
> -  lsrtags = "contemporary-notation, pitches, staff-notation,
> -  version-specific, non-traditional, world-music"
> +  lsrtags = "contemporary-notation, pitches, staff-notation,
> version-specific, non-traditional, world-music"
>
> which is a really, really, really bad idea since obviously makelsr runs
> are not cherry-picked, so this fix should have been made in a separate
> commit.  Took me a few hours (after all, make doc will take about half
> an hour on my computer).  Please folks, don't pack corrective commits
> into the commits running scripts.
>
> https://codereview.appspot.com/572600048/
I don't understand what I did wrong (apologies by the way).

I often get asked to add snippets so the workflow is to test the patch 
with the snippet + makelsr (obviously else make doc fails) but when I 
come to push, the makelsr 'run' is always in a separate commit.

I thought that was the preferred way.

If you look for 'makelsr' as a commit string you can see I've been doing 
this 'makelsr for previous commit xxx' as a separate always.

Regards

James