Message98712
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. |
|
Date |
User |
Action |
Args |
2010-02-02 04:06:43 | brian.curtin | set | recipients:
+ 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:43 | brian.curtin | set | messageid: <1265083603.77.0.330667072003.issue1578269@psf.upfronthosting.co.za> |
2010-02-02 04:06:42 | brian.curtin | link | issue1578269 messages |
2010-02-02 04:06:41 | brian.curtin | create | |
|