lilypond-user
[Top][All Lists]
Advanced

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

Re: shapeII (openlilylib) with v2.19.25


From: Urs Liska
Subject: Re: shapeII (openlilylib) with v2.19.25
Date: Tue, 13 Oct 2015 20:49:14 +0200
User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0

Am 13.10.2015 um 20:22 schrieb Peter Crighton:
> 2015-10-13 19:19 GMT+02:00 Simon Albrecht <address@hidden
> <mailto:address@hidden>>:
> 
>     On 13.10.2015 02:06, Peter Crighton wrote:
> 
>         2015-09-08 0:23 GMT+02:00 Simon Albrecht <address@hidden
>         <mailto:address@hidden> <mailto:address@hidden
>         <mailto:address@hidden>>>:
> 
> 
> 
>             Am 07.09.2015 um 22:52 schrieb Urs Liska:
> 
> 
>                 Simon,
> 
>                 please create an issue on GitHub with this information.
> 
>             <https://github.com/openlilylib/openlilylib/issues/130>
> 
>                 Should be updated with a version switch.
> 
>             Done. I included the new file in the issue.
> 
>         Is anything preventing that fix from being included into
>         openLilyLib?
> 
> 
>     Only my lack of experience with git/GitHub. I didn’t have the time
>     to delve into the topic and create a pull request myself.
> 
> 
> It’s actually quite easy in the GitHub interface if you only need to
> alter one file (unless I did something wrong … I’m far from being an
> expert either):
> – find the file you want to update & click on Edit (the pencil symbol)
> – make changes, describe them & click on “propose file change”
> – click on “Create pull request”
> – add a comment if you want & once again click on “Create pull request”
>  
> 
> 
> 
>         Or should I go ahead and create a pull request for it?
> 
> 
>     Please do so.
> 
> 
> Done.

Thank you.

Can I assume that you two have looked enough on the code so I can merge
without further testing (I'd be glad in this case)?

Looks plausible. However, I have one suggestion (not relevant to the
decision of merging the pull request but rather as a suggestion for the
future): This Pull Request has one and a half single significant change
(the switch at the end and the include at the beginning). However, the
commit *also* contains a number of modifications that are mere
reformattings. It would be preferrable if these two could be separated
into separate commits. As it is a reviewer has to carefully check all
the modified lines. If it were two commits (one "work" and one
"clean-up") it would be much more obvious.

Best
Urs



reply via email to

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