Message131411
| Author |
pitrou |
| Recipients |
eric.araujo, jhylton, mcjeff, nadeem.vawda, orsenthil, pitrou |
| Date |
2011年03月19日.13:13:08 |
| SpamBayes Score |
7.20441e-08 |
| Marked as misclassified |
No |
| Message-id |
<1300540389.11.0.00506215967743.issue10050@psf.upfronthosting.co.za> |
| In-reply-to |
| Content |
I don't think the tests should be moved from one file to the other. It's making more difficult to tell whether you have changed them or not. I think moving the tests (as well as changing the synopsis, hello Eric) are cosmetic changes that are better done in separate changesets.
Some other things:
- please do the "import tempfile" at the top-level. Imports from functions are generally frown upon, unless otherwise necessary.
- instead of "try ... finally: tfp.close()", you can simply write "with tfp: ..."
- I don't understand why `size` is read only when a reporthook is set, while it is always used for raising ContentTooShortError
- I'm not sure why the reporthook is called with `bs` rather than `len(block)`. I think the user is more interested in the actual number of bytes.
- you don't need to update Misc/NEWS yourself |
|