Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3110)

#11072: Add MLSD command support to ftplib

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 7 months ago by g.rodola
Modified:
3 years, 5 months ago
Reviewers:
michael.mischurow+bpo, ezio.melotti
CC:
AntoinePitrou, eric.smith, giampaolo.rodola, SilentGhost, Xip, devnull_devnull, SilentGhost, devnull_psf.upfronthosting.co.za
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Patch Set 4 #

Total comments: 12
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/ftplib.rst View 1 4 chunks +23 lines, -6 lines 5 comments Download
Lib/ftplib.py View 1 2 3 2 chunks +29 lines, -1 line 1 comment Download
Lib/test/test_ftplib.py View 1 2 3 5 chunks +85 lines, -1 line 6 comments Download

Messages

Total messages: 3
michael.mischurow+bpo_gmail.com
http://bugs.python.org/review/11072/diff/2036/3878 File Doc/library/ftplib.rst (right): http://bugs.python.org/review/11072/diff/2036/3878#newcode325 Doc/library/ftplib.rst:325: (:rfc:`3659`). If *path* is omitted the current directory is ...
3 years, 7 months ago #1
michael.mischurow+bpo_gmail.com
On 2011/03/10 14:20:36, SilentGhost wrote: > http://bugs.python.org/review/11072/diff/2036/3878 > File Doc/library/ftplib.rst (right): > > http://bugs.python.org/review/11072/diff/2036/3878#newcode325 > ...
3 years, 7 months ago #2
ezio.melotti
3 years, 5 months ago #3
http://bugs.python.org/review/11072/diff/2036/3878
File Doc/library/ftplib.rst (right):

http://bugs.python.org/review/11072/diff/2036/3878#newcode327
Doc/library/ftplib.rst:327: (e.g. *["type", "size", "perm"]*).  Return a
generator object yielding a
This should be ``["type", "size", "perm"]``.

http://bugs.python.org/review/11072/diff/2036/3878#newcode343
Doc/library/ftplib.rst:343: .. deprecated:: 3.3 use :meth:`mlsd` instead
IIRC deprecation sentences should end with a '.' and possibly be capitalized
properly too.

http://bugs.python.org/review/11072/diff/2036/3880
File Lib/test/test_ftplib.py (right):

http://bugs.python.org/review/11072/diff/2036/3880#newcode585
Lib/test/test_ftplib.py:585: list(self.client.mlsd(path='/', facts=['size',
'type']))
What are these testing exactly? That the result of mlsd() can be converted to a
list without raising errors? Maybe adding a short comment (or an assertEqual
that checks the return value) would be better.

http://bugs.python.org/review/11072/diff/2036/3880#newcode629
Lib/test/test_ftplib.py:629: [self.assertTrue(x.islower()) for x in
facts.keys()]
On 2011/03/10 14:20:36, SilentGhost wrote:
> [self.assertTrue(x.islower()) for x in facts] would suffice.

for x in facts:
    self.assertTrue(x.islower())
would be even better, since here we don't need any list.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7