Code review comment for lp:~jameinel/bzr/2.1-static-tuple-chk-map

Revision history for this message
John A Meinel (jameinel) wrote :

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

Andrew Bennetts wrote:
> Review: Approve
> Looks ok, it's mostly mechanical despite the large line count. A couple of questions:
>
> 772 - self.parent_id_basename_to_file_id.key())
> 773 + (self.parent_id_basename_to_file_id.key()[0],))
>
> That change (and the other similar ones looks a bit odd.
>
> Oh, is that how you're casting a StaticTuple to a tuple for string formatting? I would have thought "x.as_tuple()" would be about as fast as "(x[0],)", and it's certainly more readable. I suppose it is noticeably slower?
>
> 1115 -class TestSearchKeyFuncs(tests.TestCase):
>
> Why delete this TestCase?

I'll mention, I'm not particularly comfortable landing this until we
sort how to get PQM actually failing appropriately again. I'm pretty
sure it is good, and I'll run the tests here, but it is a more-invasive
change. I suppose Babune will give me the real fallout?

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkrlAjIACgkQJdeBCYSNAANgyQCZARSGYStiFrFUxiNnSsyoW2Si
MzoAn10O3EG6v2BN00kMZrRApjZ7SZ3e
=pRWD
-----END PGP SIGNATURE-----

« Back to merge proposal