A trivial patch to shutil.destinsrc() - Code Review (original) (raw)

Issue 841: A trivial patch to shutil.destinsrc() (Closed)

Can't Edit Can't Publish+Mail Start Review Created: 17 years ago by Raghu Modified: 13 years, 6 months ago Reviewers: Base URL: http://svn.python.org/view/\*checkout\*/python/trunk/ Visibility: Public.	Description shutil.destinsrc() can return wrong result if src is merely substring of destination (even though it is not a subdirectory). This has been submitted in the tracker some time back but tracker is not accessible right now so I uploaded local patch file (I did regenerate against latest trunk). Patch Set 1# Created: 17 years ago Download[raw] [tar.bz2] Unified diffs Side-by-side diffs Delta from patch set Stats (+29 lines, -1 line) Patch Lib/shutil.py View 1 chunk +7 lines, -1 line 4 comments Download Lib/test/test_shutil.py View 1 chunk +22 lines, -0 lines 0 comments Download Messages Total messages: 4 Expand All Messages | Collapse All Messages Benjamin http://codereview.appspot.com/841/diff/1/21 File Lib/shutil.py (right): http://codereview.appspot.com/841/diff/1/21#newcode230 Line 230: def destinsrc(src, dst): Since this is not a ... 17 years ago (2008-05-02 21:24:26 UTC)#1 http://codereview.appspot.com/841/diff/1/21 File Lib/shutil.py (right): http://codereview.appspot.com/841/diff/1/21#newcode230 Line 230: def destinsrc(src, dst): Since this is not a public API, shouldn't it get a "_" in front? Sign in to reply to this message. Raghu http://codereview.appspot.com/841/diff/1/21 File Lib/shutil.py (right): http://codereview.appspot.com/841/diff/1/21#newcode230 Line 230: def destinsrc(src, dst): On 2008/05/02 21:24:26, musiccomposition wrote: ... 17 years ago (2008-05-03 19:09:34 UTC)#2 http://codereview.appspot.com/841/diff/1/21 File Lib/shutil.py (right): http://codereview.appspot.com/841/diff/1/21#newcode230 Line 230: def destinsrc(src, dst): On 2008/05/02 21:24:26, musiccomposition wrote: > Since this is not a public API, shouldn't it get a "_" in front? I agree. The only reason I added test cases for this non-public API directly is because it is very difficult to test it from within move(). Sign in to reply to this message. Benjamin http://codereview.appspot.com/841/diff/1/21 File Lib/shutil.py (right): http://codereview.appspot.com/841/diff/1/21#newcode230 Line 230: def destinsrc(src, dst): On 2008/05/03 19:09:34, draghuram wrote: ... 17 years ago (2008-05-03 19:20:16 UTC)#3 http://codereview.appspot.com/841/diff/1/21 File Lib/shutil.py (right): http://codereview.appspot.com/841/diff/1/21#newcode230 Line 230: def destinsrc(src, dst): On 2008/05/03 19:09:34, draghuram wrote: > On 2008/05/02 21:24:26, musiccomposition wrote: > > Since this is not a public API, shouldn't it get a "_" in front? > > I agree. The only reason I added test cases for this non-public API directly is > because it is very difficult to test it from within move(). > That's fine. You're allowed to (and should) test non-public APIs if it helps verify the sanity of the public ones. http://codereview.appspot.com/841/diff/1/21#newcode230 Line 230: def destinsrc(src, dst): On 2008/05/03 19:09:34, draghuram wrote: > On 2008/05/02 21:24:26, musiccomposition wrote: > > Since this is not a public API, shouldn't it get a "_" in front? > > I agree. The only reason I added test cases for this non-public API directly is > because it is very difficult to test it from within move(). > Done. Sign in to reply to this message. GvR Hi draghuram, I've added wordwrap to the issue description and message body. Let me know ... 17 years ago (2008-05-05 16:58:13 UTC)#4 Hi draghuram, I've added wordwrap to the issue description and message body. Let me know if this is what you wanted. Sign in to reply to this message. Expand All Messages	Collapse All Messages

Issue 841: A trivial patch to shutil.destinsrc() (Closed)
Created 17 years ago by Raghu
Modified 13 years, 6 months ago
Reviewers:
Base URL: http://svn.python.org/view/\*checkout\*/python/trunk/
Comments: 4