On Mon 22 Jun 2009 at 08:26AM, Rich Burridge wrote:
> 
> Hi all,
> 
> Still looking for code reviewers on some of these outstanding pkg bug fixes.
> Down to three webrevs now.
> 
> 
> 4134 pkg operations should report operation sizes
> Bugid: http://defect.opensolaris.org/bz/show_bug.cgi?id=4134
> 
> 7736 image plan should provide space requirements for operation
> Bugid: http://defect.opensolaris.org/bz/show_bug.cgi?id=7736
> 
> Last message: 
> http://mail.opensolaris.org/pipermail/pkg-discuss/2009-June/014371.html
> Last webrev: http://cr.opensolaris.org/~richb/pkg-4134-v4/
> 

Can you include sample output, please?  (In general, if you are
changing messaging, I always want sample output).

t_pkg_install.py: I'm not a big fan of interpreting informational
messages in the test suite-- could this be done as an api
test instead?

So you've laid a bunch of track with "opstats" but we're not using
most of it, right?  Wouldn't it make sense to have a
"ondisk_size()" (or ondisk_delta() perhaps) as an analog to
transfer_size()?

We have a package plan level "get_xferstats()".  Why is this
code in imageplan?  Wouldn't it be better to have
pp.get_opstats(), and then aggregate the results at the
top level?

Also, what will happen in the case of removal?  Will estimated
space required be negative?  Perhaps the messaging should be
smarter in that case.

Why did you choose to have ip.show_size() do the messaging,
whereas ip.display() returns a string?  Why isn't the size
computation included in the output of ip.display()?  It seems
a little wedged in.  I'm not saying the other way is correct,
but rather that it should be uniform.

        -dp

-- 
Daniel Price, Solaris Kernel Engineering    http://blogs.sun.com/dp
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to