Bart Smaalders wrote:
> Bart Smaalders wrote:
>> A bit tricky in spots... note that the alternate versions
>> of svcprop and svcadm invoked during testing are defined
>> as part of the test procedure.
>>
>> - Bart
>>
>
> It must be time to go home... here's the url:
>
> http://cr.opensolaris.org/~barts/triggers/
Need a man page update to man/pkg.1.txt for the -D/--debug options.
client.py:
nitpick:
line 1805: s/--debug/%s/ and then add opt as one of the variables
to be formatted (that way whichever option they used will show up, e.g.
-D or --debug, though I wonder if that is good for localisation...)
line 1805: > 80 characters
actuator.py:
line 1: #!/usr/bin/python2.4
lines 34-36: should these be private instead (i.e. __attr)?
lines 115, 119, 128: drop
line 146ff, 248: hard-coded path to /usr/bin/svcadm, could these be
put into a constant at the top of the module instead? same goes for a
later reference to svcprop.
line 160: s/""" /"""/
line 253: need a newline here
line 267: note, this will expose open file handles to the subprocess,
and the close_fds parameter will be slow because it's poorly implemented
in Python's core
debugvalues.py:
line 1: #!/usr/bin/python2.4
image.py:
line 185: where is this attribute used? isn't this redundant with
the new class you've added?
imageplan.py:
line 117: is the str() needed given the '%s' ?
line 667: > 80 characters
line 675: > 80 characters
line 707ff: do you need to del self.actuators here?
t_actuators.py:
lines 233-234: should these be commented?
line 251: missing spaces around '='
line 268: s/se/see/
lines 275, 284, 304: missing spaces around '='
solaris.py:
line 976: need a space after ':'
Cheers,
--
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
