Shawn Walker wrote:
> https://cr.opensolaris.org/action/browse/pkg/swalker/pkg-7145863/webrev/
_varcet.c:
- line 70: this could easily be reversed, with a continue statement.
_actions.c:
- line 496: don't you need a Py_DECREF(hash) here, too? And probably
before line 476, with the appropriate test for non-NULL and non-None?
- In addition to Ed's comments on the decrefs in init_actions(), I'd add
that cache_class() might need to decref all of the previous action
class objects to be absolutely correct. Though I'll hazard that this
only happens when things are horribly wrong and we'll exit shortly
anyway.
attribute.py:
- line 62: this comment doesn't make sense anymore.
depend.py:
- line 42-50: sorted by rough order of occurrence, in order to make the
test on line 105 faster, I assume? Would a set be equally fast? If
not, a comment would be good.
- line 51: dedented four columns
file.py:
- line 592: is there no way to have "__init__ = types...." in the class
body?
generic.py:
- line 276: maybe an assert() would be appropriate here?
- line 420: AttribtueError -> AttributeError
- line 1099 (removed): this allows us to have variant.arch=x and
variant.arch=y on the same action now?
pkg_solver.py:
- line 1054: use a generator expression instead of a list comprehension?
- line 1091: no need for continuation character
facet.py:
- line 74: I think that you mean for "this" to reference "__getitem__",
but I think that normal English usage would have it reference the
situation that _allow_facet bypasses it.
manifest.py:
- line 361: why all the changes from strip to rstrip? I would think it'd
be more critical to lstrip than to rstrip, and if the action parsing
code doesn't care about leading whitespace (it's not clear to me that
it doesn't), perhaps we can just get rid of the strips entirely?
- line 609: why just for hardlinks now?
pkgdep.py:
- line 134ff: while I understand that try/except is faster than
setdefault, the latter is a nicer idiom, and this is not a
speed-critical bit of code (as far as I can tell).
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
