Brock Pytlik wrote:
> https://cr.opensolaris.org/action/browse/pkg/bpytlik/7139940-v1
image.py:
- line 2362: now that has_manifest() is doing something more interesting,
we should have a doc string for it.
- line 2368: Is the is_pkg_installed() test sufficiently faster than the
manifest verification test that we really need it? I would think that
if we had a corrupt manifest, we might want to do the verification,
regardless of whether the package was installed.
- line 2438: couldn't you just call has_manifest()?
transport.py:
- I would put this change into image.py instead -- have a private
__verify_manifest() that called _verify_manifest(), caught the
exception, and returned true or false.
dependencies.py:
- I'm assuming this change is purely for performance, and not really
related to either bug?
- line 1647: "a" -> "an"
- line 1653: "fmri's" -> "fmris"
pkg5unittest.py
- line 2222, 2264: these could be commonized, no? And why test against
need_ro_data? Do all the need_ro_data tests do signing, too? Or is it
just low-enough overhead that it's okay to do to all tests where
need_ro_data is True?
I don't understand the "PerTestRepo" class naming scheme. Why do some
files get new classes and others don't?
t_change_variant.py:
- line 484: the parens around self.pkg_shared don't mean anything here; a
1-tuple needs a trailing comma, but you can pass just the single
string. (Ditto in some of the other files.)
- I'm not sure how this test is supposed to work, though; would the
second call to change-variant succeed with the current code?
t_fix.py:
- line 330: "runing" -> "running"
- line 336: why this first fix? Isn't this always a no-op?
t_pkg_contents.py:
- line 309, 310: editorial comment: pkg contents -t <type> should
probably return 1 if there is no action of type <type> in the package.
t_pkg_info.py:
- line 649: is this really guaranteed?
- You should be able to put the license mangling into a common function.
t_pkg_install.py:
- line 1013: I'm not sure that this ordering is guaranteed.
- line 1016: I wouldn't guarantee that the rebuild would succeed with
this modification. Perhaps you could just rewrite a valid manifest on
top of m_path that simply omits the /usr/bin/cat line?
Thanks,
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
