Tim Foster wrote:
> http://cr.opensolaris.org/~timf/pkgdepend-smf
smf_manifest.py:
- Docstrings shouldn't start or end with a space (though the terminal
triple-quote can be on its own line if it won't fit in the 80 character
limit).
- You should be wrapping strings in _() so they can be translated. Any
strings that take more than one argument should use dictionaries
instead of lists:
_("Problem resolving %(fmri)s: %(err)s") % locals()
or
_("Problem resolving %(fmri)s: %(err)s") % {"fmri": fmri, "err": err}
- line 64: you shouldn't need this declaration here, as you're not
modifying the name binding at all.
- line 236ff: why not just
try:
fmris = set(get_smf_dep...)
except:
...
- line 249: bad indent
- line 247ff: the flow in this for loop is weird. You set manifest to
None so that you can test it for that on 253, but at that point you'd
append to the elist twice.
- line 267: why do this line and the next in two statements instead of
one:
pkg_attrs = {
"opensolaris.smf_fmri": instance_mf.keys()
}
- line 268: I'd change this to "opensolaris.smf.fmri". In general, we
try to avoid underscores in attribute names. I might suggest
opensolaris.smf-fmri (which is the usual "fix"), but I could see other
SMF-related metadata being added.
- line 297: could you file a bug for this, please? There are a bunch of
others that have the same problem.
- line 342: won't this give you an absolute path, rather than one
relative to the image (or proto area) root?
- line 361: specifically, when delivering instances of a single service
in separate files is supported.
t_dependencies.py:
- line 52: This doesn't cause all the smf_manifests tests to be re-run?
importer.py:
- line 174: once this if triggers, you can break out of the loop. In
fact, you can set bundle and then break out, and not bother with
bundle_data.
It might be a useful enhancement to allow search to find appropriate
subsets of SMF FMRI names. svc:/network/physical:nwam would be found by
doing a search for any of:
nwam
physical
physical:nwam
network/physical
network/physical:nwam
svc:/network/physical
svc:/network/physical:nwam
The instance name "default" could be a special case that's ignored.
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
