[email protected] wrote:
On Sun, Dec 06, 2009 at 08:36:47PM -0600, Shawn Walker wrote:
Greetings,

The following webrev contains fixes for the following issues:

  13110 image catalog rebuild could be faster
  12991 pkg image-create can trigger excessive number of stat calls

webrev:
http://cr.opensolaris.org/~swalker/pkg-13110/

Just a few nits:

catalog.py:

  - lines 589 and 590:  You've changed line 590 to check for None
    instead of True/False; however, line 589 hasn't been changed.  If
    pfmri is None it cannot have a pfmri.publisher.  Is there ever a
    situation where pfmri can be false but have a publisher?  If not,
    should the check on 589 be 'if not (pfmri or pub)' instead?

I've changed this.

  - Is there something special about how we tell if a pfmri exists?
    There are a bunch of places where 'if pfmri' has been changed to 'if
    pfmri is not None' and 'if not pfmri.publisher' was changed to 'if
    pfmri.publisher is None.'  This seems counterintuitive, which is why
    I'm curious about this part of the change.

The best explanation (assuming it is accurate) I've found is here:

http://stackoverflow.com/questions/100732/why-is-if-not-someobj-better-than-if-someobj-none-in-python

In short, "is None" is the most efficient comparison test for an object. Using a simple boolean test causes python to perform an extra conversion.

It also prevents silly bugs where a False value other than None has been specified. So I test explicitly for None since that is the only "False" case I'm expecting.

Cheers,
--
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to