On 08/05/11 16:32, Edward Pilatowicz wrote:
On Tue, Aug 02, 2011 at 06:37:22PM -0700, Brock Pytlik wrote:
On 07/22/11 14:25, Brock Pytlik wrote:
Webrev:
https://cr.opensolaris.org/action/browse/pkg/bpytlik/18439-v1
New webrev:
https://cr.opensolaris.org/action/browse/pkg/bpytlik/18439-v2
sorry for the delay.
i still need to review the tests.
comments on the code are below.
ed
----------
general
- i thought that the last time we talked about this we had agreed on
putting a copy of the "parsable_version" into the progress tracker
class? if we did this we wouldn't have to add an extra parameter to
all the linked image functions, and we wouldn't have to scan the
arguments in __pkg_cmd().
----------
src/modules/client/api.py
- in __plan_op() this comment seems wrong:
Eventually these will yield plan descriptions objects as well.
i thought eventually we'd yield plan descriptions objects instead.
- in __plan_op() we should not plan all children and then return the
results. instead do_recurse() should yield results as they become
available.
- all the gen_plan_*() functions have the following comments, which are
now wrong:
This is a generator function that returns PlanDescription objects
- i don't think we should bother with parsable output mode for
detach_linked_children() since that will never change any packages.
(it shouldn't even be going through the solver, see bug 18552.)
- the detach_linked_children() docstring describes the tuple returned by
linked image operations. please update this for the new value your
adding.
- instead of adding the None p_dict value to the return code in
audit_linked() could you add it in audit_self()? (just for
consistency so that all the LI interfaces return the same tuple.)
----------
src/modules/client/imageplan.py
- should mediators() be __mediators()? (is it accessed outside of the
class?)
No, it's accessed outside of the class. In fact, it's used by the
PlanDescription object.
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
