Code review comment for lp:~luoyonggang/subvertpy/python2

2012/3/28 Jelmer Vernooij <email address hidden>

> Review: Needs Fixing
>
> Moving away from os.tmpfile() to NamedTemporaryFile (or something similar)
> seems reasonable to me, especially given os.tmpfile()'s status.
>
> However:
> * We can't use b"" - subvertpy is compatible with 2.4 which AFAIK doesn't
> have b"".
>
For python 3

> * Please don't mix formatting with code changes - it makes it hard to
> actually see what's going on.
>
It's inconsistent mix up tab and space. And it's not so much code.

> * There is no need to explicitly close the apr file handles - that's done
> when their pool is cleaned up AFAIK.

 * Please don't reopen files in apr_file_from_object(); this may not always
> be possible
>
That's the key to get diff works under windows.
Because the file handle can not directly covert to apr_file_t.

--
> https://code.launchpad.net/~luoyonggang/subvertpy/python2/+merge/99708
> You are the owner of lp:~luoyonggang/subvertpy/python2.
>

--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo