[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