On Thu, Oct 09, 2008 at 08:33:01PM -0700, Bart Smaalders wrote:
> http://cr.opensolaris.org/~barts/triggers/
Looks nice; simpler than I thought it was going to be.
client.py:
- line 1880: "-D" should just be "D".
- line 1884: Could you put a comment here that although -D is handled
now, -R is handled below? Giving a reason would be good, too, but I
can't think of a concise explanation. Indeed, -R handling probably
could be brought up here, which would allow us to error out when saying
"pkg -R /foo image-create", which is nonsensical.
actuator.py:
- I'd really love to see a bit more commentary on what actuators are, why
they're used, etc. Perhaps you can convince Stephen to include that in
his manpage wad? :)
- Please be sure to remove all trailing spaces and tabs.
- line 27: subclass from object
- line 53: space before =
- line 72: calls to str() are extraneous.
- line 105: I'd just call this "path", I think.
- line 148, 166: you could do "tuple(suspend_fmris | tmp_suspend_fmris)"
- line 247: "smffmri" is perhaps more appropriately "svcfmri"
- line 251, 252: four space indent from previous line
- line 251: No need for outer parens?
- line 253: same indentation as line 250.
- line 257: Do you use this anywhere?
- line 272: drop str()
debugvalues.py:
- line 27: why not subclass dict? See "Subclassing built-in types" at
http://www.python.org/download/releases/2.2.3/descrintro/
image.py:
- The only change here is to add a space at the end of a line. Just
remove this file from the wad.
run.py:
- This file doesn't have any change in it. Confirm that it's not in the
changeset before you push.
distro-import/86/*:
- These should probably go into 00, no?
reboot:
- line 3: I'd love to see a mechanism so that we didn't have to tag a
bunch of files just to override a tag we don't want to see. Given that
missing values are illegal, maybe you could use that syntax to delete a
tag. Doesn't have to be this wad.
- line 7: Why not just put this into SUNWcsl? What about the 64-bit
version? What about the hwcap versions in /usr/lib/libc?
solaris.py:
- line 974: no need for semicolon
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
