This is a fairly surface review -- a couple of high-level items and a bunch of teeny nits.
Brock Pytlik wrote: > http://cr.opensolaris.org/~bpytlik/ips-11611-v4/ The signature action attributes -- "pkg.sig.X" should just be "X". Unprefixed attributes are owned by the action, while "pkg." prefixed attributes are platform-independent, non-action attributes that the packaging system owns. That'll also allow you to expand some of the names so they're more readable -- "alg" -> "algorithm" Can a manifest re-opened by an append operation have anything added to it other than signature actions? Line 109 in server/transaction.py seems to indicate that you make that distinction, but "non_sig" isn't anywhere else in your patch, so perhaps that's dead code? signed_manifests.txt: - line 145: pkg.sig.existing_pkg_vars doesn't appear anywhere in the code. - line 190, et al: "atter" -> "attr" - line 237: "manfiest" -> "manifest" man/pkg.1.txt: - line 596, et al: underscores instead of spaces when you're naming a single object, as in previous options man/pkgsign.1.txt: - line 10: it's a bit confusing to have one or more packages specified by operands, but all packages specified by an option. Perhaps remove --sign-all and accept "*" as an argument representing all packages? - line 27: "manfiest" -> "manifest" - line 31: "verifing" -> "verifying" - line 42: this downloads and re-publishes, correct? It might be useful to explain why you'd want to do this, particularly since you're moving a lot of data around this way. Does this publish at a new timestamp? - line 50: is the catalog changed by adding a signature action? I didn't see any changes to the catalog code, and I don't see signature actions referred to in the existing catalog code. actions/generic.py: - line 82: instead of "has_data_for_publication", how about "payload" or "has_payload"? "payladen"? manifest.py: - line 415: unnecessary publish/transaction.py: - line 153, 266: "additional" server/transaction.py: - line 265: what's XXX here, and why aren't you doing it in __init__? - line 483: pkg.chain_certs.sizes doesn't seem to be used anywhere else. package%2Fpkg.p5m: - line 262: the first dash in the name should be a slash Danek _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
