Hi Alok,
Per usual, if my changes creep beyond PEP8 scope, just say so.
Thanks,
Keith
General:
Single line docstrings should be on one line, including the
opening/trailing quotes
Multi-line docstrings should have a blank line before the closing quotes
transfer_defs.py:
General: May benefit from an __all__ var. (Probably out of scope)
116-117: Please align these with the open bracket on 115 if possible
transfer_mod.py:
General: Some of your docstrings got misaligned after switching from
tabs to spaces, it seems
118/123: Just set "self.handle = None", no need to add an extra init
parameter
171 & 179: I question the need for these methods, but I'll leave that
for another day...
232: Change to "while True:"
264: Seems to be an extra space after output
258-279: (Non PEP8): We don't ever seem to close output, do we need to?
302,357: Just to be sure, do we have bugs to capture these TODO?
305-315: Trailing slashes not necessary, but can be left in if they
improve readability. Your call here.
330,338,350: "if self.log_handler is not None:"
452: "if patt is not None and patt.startswith('!'):"
453,456, : negate should be a bool (set to True/False)
473-476: use "is None" or "is not None"
497: "if patt is not None:"
535-570 or so: The indentation here indicates and long 'if' statements
indicates this might benefit from refactoring either now or later.
Regardless, convert to "is None" and "is not None" where appropriate.
688: "is not None"
740: "is not None"
749-753: "while True:" - better yet, use:
char = True:
while char:
char = pipe.stdout.read(1)
self.check_abort()
936: "is not None"
994: "is not None"
1038: "is not None"
1128: "is not None"
1225: "is not None"
1272: "is not None"
1424: I believe this line is redundant. The function that
set_py_callback references in libtransfer does a PyCallable_Check() on
the arg passed in, which will fail if None is given?
1416-1468: For Python2.6, please convert this from
try/(try/except)/finally to try/except/finally.
1461: Bare except clauses are generally inappropriate in methods,
however, given the current usage scenarios for tm_perform_transfer, this
is fine.
Alok Aggarwal wrote:
> Please review my changes to make libtransfer PEP8
> compliant.
>
> http://cr.opensolaris.org/~aalok/libtransfer-pep8/
>
> The changes are currently being tested but they get
> a 10 from pylint.
>
> Thanks,
> Alok
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
