Code review comment for lp:~jameinel/bzr/2.1.0rc1-set-mtime

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

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

Martin [gz] wrote:
> Review: Approve
>> Since there is precedent, we could probably go the ..._thunk() route.
>
> Well, my point was more that there's all sorts of different precedent, which is why I get confused.
>
>> I think I've decided to go a different way, which is to just use
>> 'os.utime()' for all code paths
>>
>> In my timing tests, I can't find a benefit to custom extensions. And as
>> such, the reasonable default is to use the less complex implementation.
>>
>> I've now pushed up that version. It certainly is a lot simpler overall.
>
> Okay, this sounds like a good choice.

So I feel like this has gotten a decent amount of review, even if Martin
<gz> hasn't given an official rating.

However, I was hoping to hear from Scott as to whether it actually helps.

Anyone care to give an official "approve" ?

John
=:->

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

iEYEARECAAYFAktFF/QACgkQJdeBCYSNAAPqSwCeJtHA7TPBtqTGTmRRMWM/mdOQ
Qz8AmwUI63rBa4+MgbyY9i/1Gm8V6Cwp
=D/tr
-----END PGP SIGNATURE-----

« Back to merge proposal