Shawn Walker wrote:
Greetings,
The following webrev contains changes and fixes for the following
issues and RFEs:
5872 List APIs required
12929 image.py:get_publisher_ranks can traceback after older client
configuration changes
12946 pkg list should provide way to list only newest versions of
all packages
webrev:
http://cr.opensolaris.org/~swalker/pkg-list/
[snip]
High level comment, I'm not sold on the idea of "
Packages that do not apply to the current
image (such as those for a different architecture or that are
not applicable to the current zone type) will never be listed.
"
I'd kinda like an option to say, no, I really mean ALL packages. Mostly, I'm
thinking about a case where a user has two images in two
different situations (one on sparc, one on x86 or one that's a global zone and
one that's a non-global zone) and one is just really
hosed and they're trying to use the other to gather information about what
packages are available that might be missing.
client.py:
I'm not sure the division between client and api here is quite right. I
think more of the logic here should be moved into the api.
For example, the items on lines 294-308 I would think should be within
the logic for get_pkg_list? Wouldn't the gui want to refresh when the
cli would?
I know it's a nit, but could you rename pkg_list to pkg_selection or
something like that?
324: I'd prefer we make this function (get_preferred_publisher) part of
the api instead of leaving this stray reference directly to the image
object which I thought we wanted to remove from clients of the api.
Eventually, I'd like us to pass a api_inst instead of an image object to
functions like list_inventory or search or list_contents so that we
don't have this dual mode of image and api objects.
generic.py:
117-120: Why the duplication of code here?
catalog.py:
1475-1482: Just curious why the change to not filter out the facet or
variant actions?
version.py:
468-484: why not return False to break out early if any of these tests fail?
For example, instead of lines 465-469, why not:
if not ((other.release and self.release and
other.release.is_subsequence(self.release)) or not other.release or
other.release == '*'):
return False
I guess with all the other performance changes, I'm surprised this one
wasn't made.
508: extra blank line?
532-534: Could you rewrite this as "Takes an assumed valid... and splits
it into its components ..."? I read this sentence 3 times before I read
it with the correct meaning.
t_pkg_list:
Given that we know have t_api_list, are there tests her that could be
removed, or moved into t_api_list which is likely faster? (I'm fine with
this just being filed as a bug for future consideration.)
Thanks for getting this speedy,
Brock
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
