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: Needs Information
> Can you make the api take a file object instead? (In other words, call f.fileno() in the extension function). That would then mean I can implement it on non-C based pythons.
>
> Is there some reason os.utime can't be used?

#1, I didn't find it while looking around.
#2, it works solely on paths, rather than the open file handle, but I'm
happy to use it as a fallback.
#3 The reason to use use a fileno is because both code paths needed that
anyway, and I thought you might have a file opened by "os.open()" rather
than "open()". However, if it makes it better for other systems, I can
certainly go back to passing the file object itself.

I suppose one concern is that people may pass "StringIO()" etc to the
function, but it doesn't really matter.

I'll update accordingly so you can get another look at it before it gets
merged. Thanks for the pointer to os.utime. I swear I went through the
"os" documentation quite thoroughly and didn't find it. But I'll admit
to looking for one that took a file object/file descriptor and not just
a path.

John
=:->

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

iEYEARECAAYFAktEwy0ACgkQJdeBCYSNAAMl1QCeI69v+UjivelNxVeVsrxzXPpo
NfoAn30jWbmd9C3ORL2bkFoyrJczorI1
=P8ov
-----END PGP SIGNATURE-----

« Back to merge proposal