[issue1578269] Add os.link() and os.symlink() and os.path.islink() support for Windows

Brian Curtin report at bugs.python.org
Tue Feb 2 05:06:43 CET 2010


Brian Curtin <curtin at acm.org> added the comment:

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.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue1578269>
_______________________________________


More information about the Python-bugs-list mailing list