Shawn Walker wrote:
Greetings,
[snip]
webrev:
http://cr.opensolaris.org/~swalker/pkg-cat-p2/
[snip]
Cheers,
client.py:
356: Is there a reason to use the string "known" here instead of a constant?
520: Why is the FileInUseException case being removed?
Comments on the change from passing in img_dir to img:
I think this is going the wrong direction. My suggestion is to instead
do the __api_alloc around line 2484. Then, if a function uses the api
object, pass it in, otherwise grab the root from the api and pass that
in (until we finish moving all the functions over to the api, then we
can stop doing that). Is there a reason not to handle things that way?
api.py:
I realize this is just a temporary arrangement, but having a required
argument of img_path, and optional one of img, and having img override
img_path as if it didn't exist, seems wrong and broken to me. If we need
to be able to pass an image into __init__ (I'm not sure why we need to
yet, but I'm still working through things), I suggest we allow either an
image, or an image path to be passed as the first argument to init. The
init function can look at the type of what it was handed and decide what
to do appropriately.
400-403: I think this can be written instead as:
[ p.get_pkg_stem(anarchy=self.__img.is_pkg_preferred(p))
for p in self.__img.gen_installed_pkgs(anarchy=False)]
image.py:
Is there a reason we've removed the validation option for
refresh_publishers?
897-898: Shouldn't these be in separate try blocks? If the state was
marked as preferred, but not installed, setting this to be uninstalled
wouldn't remove preferred. Also, if you're just going to pass on the
KeyException, you should probably use discard instead of remove.
I don't understand the comment on 917-918
2276: Is there a reason to make this change?
imageplan.py:
349: is there a reason to change from f, st to res?
publisher.py:
946: why do the import here?
General question, what's a "meta_root"?
manifest.py:
468-469: unfinished comment I think?
481-482: why not just compare signatures with self.signatures?
server/catalog:
We could probably just remove the checks for .pag and .dir? Those
haven't existed since the 2008.05 release.
1151: what are the possible classes that could be coming in here?
pull.py:
Am I right that this means that pkgrecv isn't moving to the new catalog
format (yet?)
Things I haven't looked at yet:
catalog.py, test cases
Brock
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
