Hi Shawn,
On Wed, Jul 07, 2010 at 06:58:05PM -0700, Shawn Walker wrote:
> Greetings,
>
> The following webrev contains changes for the following issues:
>
> 16238 client api should ignore invalid and not yet supported packages
> 1142 match processing code replicated in various places
>
> webrev:
> http://cr.opensolaris.org/~swalker/pkg-16328/
This looks good. I just have a few nits:
api.py:
- line 1032 (and others): I've seen a bunch of instances of this:
while tgt is not None:
can this be re-written as?
while tgt:
Is it ever possible for this code 'ren_stems.get(stem, None)' to
return a stem that's valid but evaluates to False? If not, it's
simpler to omit the 'is' part of the check.
- lines 1348 - 1570: Similar question here. It looks like
raise_unmatched and return_fmris are only ever True or False. If
that's the case, there's no need to compare using 'is'.
- line 1530: Should this be, "if omit_package is True" instead?
- lines 1590 - 1769: Similar question again, there seem to be
gratuitious use of the 'is' comparison in this function too. Can we
let Python check that local and unsupported evaluate to False
without using is? Do dependencies and mfst have valid values that
evaluate to False but that aren't None?
transport.py:
- lines 921 and 1119: This can be simplified to 'if verified' instead.
manifest.py:
- line 296: It would simplify this code if you initialized this
to an empty list instead. That would mean that you could remove
lines 316-319. (How much overhead is there for allocating an empty
list? I would expect the yielding of results to dominate the time
spent in this function). This also means that you could change line
324 to 'if errors' since it will only return True if there are items
in the list.
I'm glad you deleted so much code from client.py and image.py. :)
-j
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
