Re: Changes.tely - reorganize entries for 2.20 release (issue 326400043 by pkx166h@gmail.com)

classic Classic list List threaded Threaded
3 messages Options
dak
Reply | Threaded
Open this post in threaded view
|

Re: Changes.tely - reorganize entries for 2.20 release (issue 326400043 by pkx166h@gmail.com)

dak

https://codereview.appspot.com/326400043/diff/60001/Documentation/changes.tely
File Documentation/changes.tely (right):

https://codereview.appspot.com/326400043/diff/60001/Documentation/changes.tely#newcode67
Documentation/changes.tely:67: hyphenated;
A few remarks after the fact (sorry for that): we are talking only about
long English pitch names here, so it is more like "In English notename
language, pitch names containing @samp{sharp} or @samp{flat} now need to
be hyphenated."

Then generally you appear to use a semicolon instead of a colon before
examples; I don't think that improves readability and it doesn't match
our style elsewhere.

There are lots of whitespace errors as well (spaces before end of line).

Try

     git log --check origin/stable/2.20

for an exhaustive list of newly introduced (according to git diff which
might consider a lot of material actually moved around as if it were
new) problems.

https://codereview.appspot.com/326400043/

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Changes.tely - reorganize entries for 2.20 release (issue 326400043 by pkx166h@gmail.com)

pkx166h
On 2017/10/01 10:57:31, dak wrote:

https://codereview.appspot.com/326400043/diff/60001/Documentation/changes.tely
> File Documentation/changes.tely (right):


https://codereview.appspot.com/326400043/diff/60001/Documentation/changes.tely#newcode67
> Documentation/changes.tely:67: hyphenated;
> A few remarks after the fact (sorry for that): we are talking only
about long
> English pitch names here, so it is more like "In English notename
language,
> pitch names containing @samp{sharp} or @samp{flat} now need to be
hyphenated."

> Then generally you appear to use a semicolon instead of a colon before
examples;
> I don't think that improves readability and it doesn't match our style
> elsewhere.

> There are lots of whitespace errors as well (spaces before end of
line).

Oh. Sorry.


> Try

>      git log --check origin/stable/2.20

> for an exhaustive list of newly introduced (according to git diff
which might
> consider a lot of material actually moved around as if it were new)
problems.

If you want, you can remove this commit and I will go back and check/fix
all the whitespace errors and recommit it.

I don't recall complaints from git about whitespace when I applied the
patch to my local copy of stable nor did I notice any excessive
whitespace chars in my editor (I use Geany).

I have closed this issue (after I commited) so I can redo a new issue,
it's not a huge deal for me.

James

https://codereview.appspot.com/326400043/

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
dak
Reply | Threaded
Open this post in threaded view
|

Re: Changes.tely - reorganize entries for 2.20 release (issue 326400043 by pkx166h@gmail.com)

dak
In reply to this post by dak
On 2017/10/01 17:11:41, pkx166h wrote:
> On 2017/10/01 10:57:31, dak wrote:
> >
https://codereview.appspot.com/326400043/diff/60001/Documentation/changes.tely
> > File Documentation/changes.tely (right):
> >
> >

https://codereview.appspot.com/326400043/diff/60001/Documentation/changes.tely#newcode67
> > Documentation/changes.tely:67: hyphenated;
> > A few remarks after the fact (sorry for that): we are talking only
about long
> > English pitch names here, so it is more like "In English notename
language,
> > pitch names containing @samp{sharp} or @samp{flat} now need to be
hyphenated."
> >
> > Then generally you appear to use a semicolon instead of a colon
before
> examples;
> > I don't think that improves readability and it doesn't match our
style
> > elsewhere.
> >
> > There are lots of whitespace errors as well (spaces before end of
line).

> Oh. Sorry.

> >
> > Try
> >
> >     git log --check origin/stable/2.20
> >
> > for an exhaustive list of newly introduced (according to git diff
which might
> > consider a lot of material actually moved around as if it were new)
problems.

> If you want, you can remove this commit and I will go back and
check/fix all the
> whitespace errors and recommit it.

No, we don't remove commits from master.  That's a recipe for disaster.
Just make some followup commit.  The whitespace alone would not need a
review I guess.

> I don't recall complaints from git about whitespace when I applied the
patch to
> my local copy of stable

Git does not as such complain about whitespace: after all, it is a legit
part of files and Git has no way to guess when it is unwanted.

> nor did I notice any excessive whitespace chars in my
> editor (I use Geany).

No idea whether it gives indication unless asked.

> I have closed this issue (after I commited) so I can redo a new issue,
it's not
> a huge deal for me.

If you go for a review, that's the clean path.  One can reopen an issue,
but that's making things a lot muddier since there already are committed
changes in master.

https://codereview.appspot.com/326400043/

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel