2013/9/26 :
> On 2013/09/26 10:51:34, janek wrote:
>> Uh, David, thanks for this explanation, but i'm afraid it was not
>> needed. If the code doesn't need a comment, just say so.
>>
>> Now, speaking in general: i don't understand that when Mike submits a
>> patch, you often complain that it's no
On 2013/09/26 10:51:34, janek wrote:
> It does not belong in the code. It might belong in the issue
tracker,
> perhaps in the commit message. Just as a record of what went wrong.
> But the code, as it is written, does not offer a temptation of
changing
> it back to the buggy previous versi
2013/9/25 :
>
>> > On 2013/09/24 07:44:44, janek wrote:
>> >> Hmm. From my point of view, this deserves some comment
>> >> (but i don't insist).
>
> Well, multiple matching regexps are messy and might call for an
> appropriate comment. But the whole point of the change is not to have
> multiple m
On 2013/09/24 22:29:56, janek wrote:
https://codereview.appspot.com/13256053/diff/1/lily/lexer.ll
File lily/lexer.ll (right):
https://codereview.appspot.com/13256053/diff/1/lily/lexer.ll#newcode588
lily/lexer.ll:588: [^|*.=$#{}\"\\ \t\n\r\f0-9][^$#{}\"\\ \t\n\r\f0-9]*
{
On 2013/09/24 16:15:
https://codereview.appspot.com/13256053/diff/1/lily/lexer.ll
File lily/lexer.ll (right):
https://codereview.appspot.com/13256053/diff/1/lily/lexer.ll#newcode588
lily/lexer.ll:588: [^|*.=$#{}\"\\ \t\n\r\f0-9][^$#{}\"\\ \t\n\r\f0-9]* {
On 2013/09/24 16:15:39, dak wrote:
On 2013/09/24 07:44:44, ja
https://codereview.appspot.com/13256053/diff/1/lily/lexer.ll
File lily/lexer.ll (right):
https://codereview.appspot.com/13256053/diff/1/lily/lexer.ll#newcode588
lily/lexer.ll:588: [^|*.=$#{}\"\\ \t\n\r\f0-9][^$#{}\"\\ \t\n\r\f0-9]* {
On 2013/09/24 07:44:44, janek wrote:
Hmm. From my point of vi
https://codereview.appspot.com/13256053/diff/1/lily/lexer.ll
File lily/lexer.ll (right):
https://codereview.appspot.com/13256053/diff/1/lily/lexer.ll#newcode588
lily/lexer.ll:588: [^|*.=$#{}\"\\ \t\n\r\f0-9][^$#{}\"\\ \t\n\r\f0-9]* {
Hmm. From my point of view, this deserves some comment (but i
Vik Reykja writes:
> On Mon, Sep 23, 2013 at 8:50 AM, wrote:
>
>> On 2013/09/22 22:30:51, Vik Reykja wrote:
>>
>>> I think a patch like this requires at least one regression test.
>>>
>>
>> Well, actually the original lexer.ll patch would more likely have called
>> for a regression test as it co
On Mon, Sep 23, 2013 at 8:50 AM, wrote:
> On 2013/09/22 22:30:51, Vik Reykja wrote:
>
>> I think a patch like this requires at least one regression test.
>>
>
> Well, actually the original lexer.ll patch would more likely have called
> for a regression test as it covers a lot more change.
>
Well
Reviewers: Vik Reykja,
Message:
On 2013/09/22 22:30:51, Vik Reykja wrote:
I think a patch like this requires at least one regression test.
Well, actually the original lexer.ll patch would more likely have called
for a regression test as it covers a lot more change.
Would you want to propose o
I think a patch like this requires at least one regression test.
https://codereview.appspot.com/13256053/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
11 matches
Mail list logo
