Author brian.curtin
Recipients amaury.forgeotdarc, asvetlov, brian.curtin, eric.smith, ggenellina, giampaolo.rodola, jafo, jaraco, lemburg, loewis, nnorwitz, r.david.murray, ssbarnea, swarren
Date 2010-02-02.04:06:41
SpamBayes Score 5.97153e-10
Marked as misclassified No
Message-id <1265083603.77.0.330667072003.issue1578269@psf.upfronthosting.co.za>
In-reply-to
Content
Most of the guts have already been reviewed, but here are some mainly minor comments on the last patch.

Lib/test/symlink_support.py
- No need to import print_function
- I'd just put the docstrings on one line
- A bunch of if tests are one-lined

Lib/test/test_win_symlink.py
- getwindowsversion now returns named members, so you can just use getwindowsversion().major for example, rather than the tuple.
- Rather than check the Windows version in the setUp, I think it would be cleaner to use the unittest.skipIf decorator on the entire WindowsSymlinkTest class.
- You might want to stack skip decorators to first skip outright if it's not Windows, and then skip if it's not Win 6 or greater -- it gets you out of checking if getwindowsversion actually exists.
- test_remove_directory_link_to_missing_target has some commented out code.

Lib/test/test_posixpath.py
- Lines 365-366 - why check has_symlink() and immediately check it again?

Modules/posixmodule.c
- Line 2740 - "//*** todo: fix this"
- C++ style comments in a few other places (nit picky, I know)
- Line 5117 - win_symlink__doc__ is double spaced and contains an extra slash at line 5121. For consistency it should probably just be single spaced.
History
Date User Action Args
2010-02-02 04:06:43brian.curtinsetrecipients: + brian.curtin, lemburg, loewis, nnorwitz, jafo, amaury.forgeotdarc, jaraco, ggenellina, eric.smith, giampaolo.rodola, swarren, r.david.murray, ssbarnea, asvetlov
2010-02-02 04:06:43brian.curtinsetmessageid: <1265083603.77.0.330667072003.issue1578269@psf.upfronthosting.co.za>
2010-02-02 04:06:42brian.curtinlinkissue1578269 messages
2010-02-02 04:06:41brian.curtincreate