[MERGE] Add a smart method method that can pull a set of revisions in a single request

Aaron Bentley aaron.bentley at utoronto.ca
Tue Sep 4 06:28:50 BST 2007


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Bennetts wrote:
>> This looks pretty good.  I still don't see a direct test for 
>> self.item_keys_introduced_by, though.  Without that, there's no 
>> justification for implementing it on RemoteRepository.
> 
> I've added a test for this.

Thanks.

bb:approve

> I've tidied up the long lines that I can; unfortunately the rather long class
> name makes it hard to avoid in a couple of spots. Let me know if you have
> specific suggestions to fix these,

Well, there's always the option of changing the class name to something
shorter.

The long line in test_is_incompatible_different_format_both_remote looks
trivial to fix.

> but I'm not too bothered (even though I
> recently switched to using two 79-column windows side-by-side in my normal vim
> usage).

Well, I think lines that are intrinsically impossible to wrap are a bad
smell.  It suggests the code isn't very readable, either.  And I'd much
rather have humans wrap code than have vim wrap a long line at display time.

> My editor highlights trailing whitespace, and I didn't see any.  I think your
> --check-style might be picking up lines that are entirely whitespace, e.g.
> between method definitions in a class, or other blank lines inside indented
> code.  This seems harmless to me, and I don't see anything in HACKING.txtg or
> PEP 8 against it.

It's true that --check-style doesn't make an exception for lines that
are entirely whitespace.  I don't understand why it should, though I've
never really understood people's desire to avoid trailing whitespace in
the first place.

>> I know we plan to get the smart server running on bazaar-vcs.org, but we 
>> shouldn't put this version on until that repository has been reconciled.
> 
> Agreed.  That shouldn't block us from merging this change though, IMO.

Of course.  Just something I mentioned in passing.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFG3O0S0F+nu1YWqI0RAn7kAJ9TUo7RQfjsFwezCwsBQ4gCc7FLYwCfVbIZ
sePB6qRtjL8WVNvJ2/xAotU=
=9FME
-----END PGP SIGNATURE-----



More information about the bazaar mailing list