Shawn Walker wrote:
> https://cr.opensolaris.org/action/browse/pkg/swalker/pkg-18440/webrev/
catalog.py:
- line 1738: I appreciate that you're refactoring this code into a single
method in this class, but we still have four nearly-identical
implementations of this scattered across three source files. I hope
we're planning on rationalizing that insanity.
Note that this doesn't really need "self", either.
- line 1743: they're functions, no, not really constant values?
- line 2827: here's another instance of something I'm positive I've seen
before. I know there are significant differences, but we have to
figure out a way (post FCS) to refactor these functions so that they're
maintainable. You might want to have a comment here explaining that
this is a copy of __get_pkg_list() from api.py, with some its
functionality removed, which is why some constructs look weird here.
And that changes here should be reflected there (and vice versa).
I think I suggested a long time ago to throw an exception at the end of
the generator to return things like the matching and non-matching
patterns here. I wonder if perhaps a better paradigm is to return
those values through parameters, and if so, why I hadn't thought of
that (or rejected it) at the time.
- line 2938: duplicate of 2929?
misc.py:
- line 1115: "other" -> "others"
pkgrepo.py:
- line 764: no need for a backslash
- line 820: so in the json formats, state is None? And hence
short_state? I'm not sure I get the distinction between those, other
than the header being different.
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
