All LGTM now. Thanks Phil!
Trevor
http://codereview.appspot.com/6454121/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
http://codereview.appspot.com/6454121/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM
http://codereview.appspot.com/6454121/diff/7/input/regression/relative-repeat.ly
File input/regression/relative-repeat.ly (right):
http://codereview.appspot.com/6454121/diff/7/input/regression/relative-repeat.ly#newcode4
input/regression/relative-repeat.ly:4: system has alll the notes with
On 2012/08/10 13:55:51, email_philholmes.net wrote:
But when you're checking the regtests, you don't see the
code at all. To make visual checking possible, there has
to be a description of what you're looking at, not what the
test does.
OK, if we accept this, someone needs to look at the cod
Please review
http://codereview.appspot.com/6454121/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Sorry I didn't notice this before.
'relative-repeat.ly' was testing something different than you thought.
http://codereview.appspot.com/6454121/diff/12002/input/regression/relative-repeat.ly
File input/regression/relative-repeat.ly (left):
http://codereview.appspot.com/6454121/diff/12002/input
Please review final mods.
http://codereview.appspot.com/6454121/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
http://codereview.appspot.com/6454121/diff/11001/input/regression/markup-user.ly
File input/regression/markup-user.ly (right):
http://codereview.appspot.com/6454121/diff/11001/input/regression/markup-user.ly#newcode22
input/regression/markup-user.ly:22: \override PaperColumn
#'keep-inside-line =
http://codereview.appspot.com/6454121/diff/11001/input/regression/relative-repeat.ly
File input/regression/relative-repeat.ly (right):
http://codereview.appspot.com/6454121/diff/11001/input/regression/relative-repeat.ly#newcode10
input/regression/relative-repeat.ly:10: \alternative { { a2_"Alt1"
Please review updated patch set.
http://codereview.appspot.com/6454121/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
- Original Message -
From:
To: ;
Cc: ;
Sent: Wednesday, August 08, 2012 7:15 PM
Subject: Re: Regtest changes phase 1 (issue 6454121)
Sorry Phil, but I don't think this is an improvement.
a) The original code comments explain much more clearly
what is being tested than do you
On Wed, Aug 08, 2012 at 09:17:38PM +0200, David Kastrup wrote:
> "Phil Holmes" writes:
>
> > Absolutely appreciate this comment. However, the theory of the
> > regtests is that by looking at the description and the image, you can
> > tell whether the regtest has been passed. No-one who looked a
"Phil Holmes" writes:
>> b) Although the marks don't affect the test they do
>> tend to mask the real tests and make it harder to see
>> what is really being tested.
>>
>> I'd leave the code alone and expand the description if
>> further clarity is needed.
>>
>> Trevor
>
>
> Absolutely appreciate
- Original Message -
From:
To: ;
Cc: ;
Sent: Wednesday, August 08, 2012 7:23 PM
Subject: Re: Regtest changes phase 1 (issue 6454121)
http://codereview.appspot.com/6454121/diff/1/input/regression/relative-repeat.ly
File input/regression/relative-repeat.ly (right):
http
On 2012/08/08 18:41:43, email_philholmes.net wrote:
However, the theory of the regtests is that by looking
at the description and the image, you can tell whether the
regtest has been passed.
Yes. To improve that you can change either the image or
the description. In this case I think changin
On 2012/08/08 18:42:44, mail_philholmes.net wrote:
I did try, and I could see no effect from \relative.
Of course. It's working correctly now. The point of
a regtest is to ensure it continues to work correctly.
http://codereview.appspot.com/6454121/
- Original Message -
From:
To: ;
Cc: ;
Sent: Wednesday, August 08, 2012 7:15 PM
Subject: Re: Regtest changes phase 1 (issue 6454121)
Sorry Phil, but I don't think this is an improvement.
a) The original code comments explain much more clearly
what is being tested than do you
http://codereview.appspot.com/6454121/diff/1/input/regression/relative-repeat.ly
File input/regression/relative-repeat.ly (right):
http://codereview.appspot.com/6454121/diff/1/input/regression/relative-repeat.ly#newcode10
input/regression/relative-repeat.ly:10: \alternative { a1_"Alt1"
e_"Alt2"
My earlier comments apply only to the first of these three
regtests. I haven't looked at the others yet.
http://codereview.appspot.com/6454121/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-deve
Sorry Phil, but I don't think this is an improvement.
a) The original code comments explain much more clearly
what is being tested than do your new marks.
b) Although the marks don't affect the test they do
tend to mask the real tests and make it harder to see
what is really being tested.
I'd l
http://codereview.appspot.com/6454121/diff/1/input/regression/context-mod-with.ly
File input/regression/context-mod-with.ly (right):
http://codereview.appspot.com/6454121/diff/1/input/regression/context-mod-with.ly#newcode4
input/regression/context-mod-with.ly:4: texidoc = "Context modifications
initial review of initial regtest changes.
http://codereview.appspot.com/6454121/diff/1/input/regression/context-mod-with.ly
File input/regression/context-mod-with.ly (right):
http://codereview.appspot.com/6454121/diff/1/input/regression/context-mod-with.ly#newcode4
input/regression/context-mod
Reviewers: Graham Percival,
Message:
Please review initial regtest updates
Description:
This is just 3 files to start with, to check the principle of adding
considerable extra descriptive text to the regtest to make it mode
apparent what is being tested. These were the 3 lowest marked regtests
23 matches
Mail list logo
