On 07/26/11 13:48, Danek Duvall wrote:
Shawn Walker wrote:
https://cr.opensolaris.org/action/browse/pkg/swalker/pkg-image-1/webrev/
pkg.1.txt:
- line 147: "... rooted at directory dir. If no directory was ..."
client.py:
- line 5819: I think I understand why you did it this way, but it seems
very strange to me to set pkg_image_used based on the environment
variable, rather than on a return value of get_default_image_root().
The way it is, it assumes a) that get_default_image_root() uses
PKG_IMAGE (which may be part of the method contract) and b) it hasn't
been removed from the environment since the call (which won't happen in
our code, but isn't impossible for other clients).
a) is part of the method contract, and b) yes, it's assuming it hasn't
been removed from environment. Is it sufficient to add a comment for that?
My initial implementation actually returned the source as a string token
(e.g. "PKG_IMAGE", "CWD", or "PLATFORM") but I wasn't comfortable making
that "interface".
api.py:
- line 4639: "may not actually be valid" probably needs a touch more
explanation. Why should a client use an invalid root as a default
location? Would they be expected to do an upwards search, or is it
that find_root() will simply handle that? (If so, we might mention
that it -- or the Image constructor -- will correct it.)
find_root() handles that, I've expanded the docstring there with this:
The ImageInterface object will use the root provided as a starting point
to find an image, searching upwards through each parent directory until
'/' is reached. The '/' may not be a valid image on systems such as
Solaris 10.
...
- line 4652: you can exit this if clause with root still None. Should it
just be set to the live root at that point?
As far as I can see, that only happens if portable.osname != "sunos",
which was my intention per comments on lines 4663-4666.
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
