On 04/15/11 05:41 PM, Edward Pilatowicz wrote:
On Wed, Mar 30, 2011 at 09:36:05PM -0700, Brock Pytlik wrote:
Here's the system repository work that Tim and I have been working
on for quite a while now.
http://cr.opensolaris.org/~bpytlik/ips-sysrepo-v1/
more comments.
ed
----------
src/modules/client/transport/repo.py
- why do this for FilesystemRepo and HTTPSRepo:
def __init__(..., proxy=None):
assert not proxy
HTTPRepo and TransportRepo (the parent classes) don't have this
__init__() parameter.
Vestigial code already removed.
----------
src/pkg/pkglint_whitelist.txt
- the change from package/pkg/update-manager to system/desktop/ldtp
seems weird...
I don't have a good explanation for why it happened, though I'm not sure
it's a "change" as much as a removal and addition. Maybe Tim has an idea?
----------
src/modules/client/imageconfig.py
- it seems you've made "preferred-authority" irrelevant, so why not just
remove it? (if it's being kept for backward compatability then a
comment to that effect somewhere would be good.) also, it seems that
all the code in write() that sets ppub can be removed since you removed
the only consumer.
You'd have to be more specific. It's kept in version 2 of image configs
for backwards compatibility.
I've pulled it out of the write function.
- when determining the sysrepo proxy url, i think we should consult:
1) environment
2) smf - system/zones-proxyd - config/proxy_host
3) smf - system/pkg/sysrepo - config/port
4) error?
3 is important (instead of just defaulting to localhost:1008).
For use in images in global zones? Sure.
- a comment explaining why you do this (which seems odd to me) would be
nice:
portable.remove(syscfg_path)
...
self.sys_cfg = ImageConfig(syscfg_path, None)
Sure. It's to remove the old configuration.
- BlendedConfig() could be easier to read if you did:
if not use_system_pub:
self.sys_cfg = NullSystemPublisher()
self.__publishers, self.added_pubs, self.removed_pubs =
self.__merge_publishers(self.img_cfg, self.sys_cfg,
pkg_counts, old_sysconfig, self.__proxy_url)
return
...
I'd rather keep it the way it is so that the call to merge_publishers is
shared.
Thanks,
Brock
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
