Ray.Allen <ysj....@gmail.com> added the comment: Thanks for reviewing!
> The name 'ensure_exist' for the new parameter strikes me as wrong, as well as > too long for something that will nearly always be set to True. The function > always ensures that the directory exists. The question is whether it is ok > that it exist previously. I strongly suggest something shorter like > 'exist_ok' as an alternative. Yes, I guess you'are right, this parameter needs to be shorter. > Does the use of 'base = support.TESTFN' ensure that the test junk gets > cleaned up? Yes, the MakedirTests.tearDown() method will try to look for the outermost directory of "@base/dir1/dir2/dir3/dir4/dir5/dir6", so it can ensure the test directory is cleaned up. > * Location of error suppression: this patches posixfile.mkdir; 1675 wraps it > with a new os.mkdir function that does the suppression. I can see an argument > for each approach. The purpose of posix_mkdir() is to simulate a shell mkdir command, so I think the function of "-p" option should goes in to this code. A wrapper makes code more complicated. > * Propagation to parent directories: this passes exist_ok to parent mkdir(); > 1675 passes exist_ok=True, so that it is never an error for parent > directories to exist. This is a change in behavior and might be bad for the > same reason we do not make exist_ok=True the default. In any case, I believe > either patch could be changed to mimic the other. It seems these two ways has the same effect, I have no opinion on which to prefer. By adopting your parameter naming suggestion and a little coding style change, I update the patch. ---------- Added file: http://bugs.python.org/file18075/mkdir_py3k_updated.diff _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue9299> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
