Edward Pilatowicz wrote:
>
> https://cr.opensolaris.org/action/browse/pkg/edp/pkg.pli.cr3/webrev.cr1.diff
Mostly nits.
client.py:
- line 1589: "parasable" -> "parsable"
- line 1614: "point" -> "pointer"?
- line 1691: do we clean up the saved plan if there's nothingtodo()?
client/api.py:
- line 39: "compleatly" -> "completely"
- line 99: if we're not using PlanDescription, why do the import, rather
than disabling the lint warning? Only place I see PlanDescription is
on lin 1147, where we refer to it as plandesc.PlanDescription, so we
probably actually don't need this import.
- line 131: if this function isn't used outside this module, then you can
just prefix it with a double underscore for privacy.
I also wouldn't call it "..._decorator"; it's like calling all your
functions "..._function".
But I wonder if it should look more like the _LockedCancelable class
(I'm not sure why that was implemented as a class rather than a
function), or whether you should go even further and simply use the
_LockedCancelable class, possibly modifying the __call__() method to
give it different behavior in some circumstances.
nrlock.py:
- line 58: docstring is identical to the one for locked(); probably could
have a couple of words added.
pkgremote.py:
- Perhaps async_call_1 and async_call_2 (& related) should be async_call
and async_waiter? Or something that doesn't make it seem like you've
only got two things going on at once?
- line 150: "create pipe" -> "create a pipe"
- line 219: "'state' an optional" -> "'state' is an optional"
- line 231ff: these will all end up printing out "None is None", which
doesn't seem useful.
- line 291, 332, 386, 400: "arguments" -> "argument dict"
- line 347: no need for parens around "rv"
misc.py:
- line 604: "represented character" -> "represented as character"
- line 690: no parens around lhs
setup.py:
- line 310: should be in alphanumeric order
t_misc.py:
- line 97: "resonable" -> "reasonable"
Thanks,
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
