[MERGE] Patience diff

John A Meinel john at arbash-meinel.com
Tue May 23 14:26:35 BST 2006


Johan Rydberg wrote:
> John A Meinel <john at arbash-meinel.com> writes:
> 
>> [comments on Aarons things]
> 
> John, when commenting on code, could you maybe try to keep the inlined
> code to a minimum?  I find it hard to navigate through your comments,
> for two reason;
> 
>   1) because of all the excessive inlined code, I'm having trouble
>      understanding exactly what you are commenting on.
>      When quoting, there is nothing wrong with minimizing the text.
>      Actually, it makes things easier to read.  If people want the
>      read the whole thread, they can do just that in their own
>      mail reader. :)
> 
>   2) are you commenting on the code above or below the comment?
> 
> You make for example this comment:
> 
>> [...]
>> +from bisect import bisect
>> +from copy import copy
>> +import difflib
>> +import time
>> +import os
>> +import sys
>>
>>
>> PEP8 here
> 
> I suppose you are commenting on the imports (ie, commenting the code
> _above_ the comment).
> 

Actually, I was commenting about below, not above. (There are some
missing whitespace).
Though now that you mention it, time should come after sys, not before.

> 
>> [...]
>> +    result = []
>> +    k = lasts[-1]
>> +    while k is not None:
>> +        result.append((btoa[k], k))
>> +        k = backpointers[k]
>> +    result.reverse()
>> +    return result
>>
>> PEP8. Also, Bram seemed to like running self tests every time the module
>> was imported. Which are what all these asserts are for.
>> They should be fast, so I'm okay with keeping them, but I did add them
>> to selftest, so it would be safe to get rid of them.
>>
>> +
>> +assert unique_lcs('', '') == []
>> +assert unique_lcs('a', 'a') == [(0, 0)]
>> +assert unique_lcs('a', 'b') == []
>> +assert unique_lcs('ab', 'ab') == [(0, 0), (1, 1)]
>> +assert unique_lcs('abcde', 'cdeab') == [(2, 0), (3, 1), (4, 2)]
>> +assert unique_lcs('cdeab', 'abcde') == [(0, 2), (1, 3), (2, 4)]
>> +assert unique_lcs('abXde', 'abYde') == [(0, 0), (1, 1), (3, 3), (4, 4)]
>> +assert unique_lcs('acbac', 'abc') == [(2, 1)]
> 
> Where as here it seems that you are commenting on the code _below_ the
> comment. (maybe with the exception of the PEP8 comment)
> 
> Or it might just be my jetlag that is causing brain damage.  Anyhow, I
> appreciate your comments, and I hope you take this the right way.
> 
> ~j

I'll try and make things simpler. The difficulty is determining how much
context is sufficient. Also, for whatever reason, Thunderbird handles
Aaron's attachments in a very weird way. It quotes his reply, but then
doesn't quote the attachment (it leaves it inline). And when I view a
reply, Thunderbird is good about coloring quoted lines, which makes it
more obvious where my comments are, so with it inline, it is less obvious.

I'm not sure if it is T-bird or Enigmail causing the attachment
problems. (I'm guessing Enigmail combined with Aaron doing inline
signatures, versus MIME signatures).
I'll try to be more careful.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060523/e78ff290/attachment.pgp 


More information about the bazaar mailing list