[email protected] wrote:
On Fri, Sep 04, 2009 at 08:43:21AM +0100, Padraig O'Briain wrote:
Brock Pytlik wrote:
Shawn Walker wrote:
[email protected] wrote:
On Thu, Sep 03, 2009 at 07:55:25AM +0100, Padraig O'Briain wrote:
On 08/27/09 10:22, Padraig O'Briain wrote:
The webrev, http://cr.opensolaris.org/~padraig/ips-10539-v1/, fixes

10539 Package Manager hangs after canceling Update All
Could someone have a look at this proposed change to api.py as it causes severe problems to the GUI?
Yes.  Brock or Shawn should probably also look at this, but here are my
suggestions.

  - lines 237 & 238:  Since you've moved self.__reset_unlock() and
  self.__activity_lock.release() out of conditional that follows this,
  it probably makes sense to remove it here too, and just let it get
  called from the common location.

  - line 240:  If you take the above suggestion, this should be an elif
    not an if, I think.
I agree. I was initially concerned that there were places that called plan_common_exception that didn't call plan_common_start, but that doesn't appear to be the case (the concern being that release() would be called without acquire() being called...).

Cheers,
Yep, I agree with the suggestions. I'd also suggest removing the # NOT REACHED comment.
I have respun the webrev: http://cr.opensolaris.org/~padraig/ips-10539-v2/

The second of my two comments doesn't appear to be addressed by this
webrev.  The conditional on line 237 (in the newest webrev) should be an
elif, so you don't log two different errors in this code path.

Sorry; missed that comment. I have spun a new webrev: http://cr.opensolaris.org/~padraig/ips-10539-v3/

Padraig


Thanks,

-j
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to