Bart Smaalders wrote:
> Another update is in place; this one correctly handles
> multiple optional/incorp dependencies on the same package in the
> same package.
>
> http://cr.opensolaris.org/~barts/incorp/
actions/depend.py:
line 42: This looks like an unused import.
line 57: s/semantics/the semantics/ s/doen't/doesn't/
line 160: s/""" returns/"""returns/
line 161: s/a /an /
client/api.py:
lines 339, 347: commented code?
lines 340, 346: extraneous whitespace?
client/api_errors.py:
line 128: I don't think I would include "pkg: " in this string here
since this may be shown by the GUI, etc. Perhaps s/pkg: the/The/ ? I
realise that all the other messages in PlanCreationException do this,
but I don't think they should either. The other exceptions don't.
client/constraint.py:
line 27: unused import?
line 30: docstring for exception appreciated; just a brief
description of what/when intended to be used
lines 43-64: Why assign to msg if you're just going to return it
later? You could just immediately return _("foo") and at the bottom
instead of an "else: assert ... return msg" have "assert ..."?
line 67: docstring for exception appreciated; just a brief
description of what/when intended to be used
line 122: s/#find/# find/
line 149-150: why not use a lambda function for constraint_combine here?
lines 155-158: the wrapping seems a bit odd here
line 163: needs wrapping
line 170: dosctring would be appreciated
lines 172-175: i've been debating whether these constraints should be
here or just module-level instead; we seem to follow the latter practice
more often (I'm not asking you to change this, just pointing it out)
lines 196-198: these lines look longer than 80 characters
line 223: s/to do to meet/necessary to satisfy/
line 227: s/required"""/required."""/
line 230: needs wrapping
line 235: commented code?
lines 243-244, 255: needs wrapping
line 248: docstring appreciated
client/image.py:
line 1602: docstring appreciated
line 1614: s/"""/."""/
line 1628: looks longer than > 80 characters
line 2091: extraneous whitespace?
line 2099: s/this/This/
client/imageplan.py:
line 182: s/return/Return/
line 183: s/yet"""/yet."""/
line 286: s/update/Update/
line 287: s/existing/an existing/
line 288: s/here/here./
line 355: extraneous whitespace added at end of line
line 699: what changed here? can you do a s/\s\+$// over this file?
cli/t_pkg_api_install.py:
line 152: assigning to res, e seems pointless here; maybe a comment
instead saying that the return value is intentionally ignored for
testing purposes?
cli/t_pkg_install.py:
lines 774, 798: s/incoprs/incorps/
lines 813-814: do we have a bug for this?
line 1626: s/don'tsee/don't see/
Cheers,
--
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
