On Wed, Apr 09, 2008 at 10:40:25PM -0700, Dan Price wrote:
> On Wed 09 Apr 2008 at 09:47PM, Danek Duvall wrote:
> > http://cr.opensolaris.org/~dduvall/pkg-bug821/
>
> 132: Is a \n needed here before the contents of stdout? It could
> be more than one line of output.
Yeah ... all of the output that the driver action currently spews could
look a lot nicer.
> 61-68: I had assumed that Popen's stdin default is /dev/null? It's
> not clear from the documentation (it says it's "None" but that
> doesn't mean much).
It goes on to explain that "None" means the descriptor is inherited from
the parent.
> Callers of __call-- perhaps you could commonize all of this
> code into __call()?
>
> 285 301 args = add_base + ("-p", i, self.attrs["name"])
> 286 - retcode = subprocess.call(args)
> 302 + retcode, stdout = self.__call(args)
> 287 303 if retcode != 0:
> 288 304 print "%s (%s) upgrade (addition of policy " \
> 289 305 "'%s') failed with return code %s" % \
> 290 306 (self.name, self.attrs["name"], i, retcode)
> 291 307 print "command run was ", args
> 308 + print "command output was", stdout.read()
>
> Becomes:
>
> self.__call(fail="%s (%s) upgrade (addition of policy " \
> "'%s') failed." % (self.name, self.attrs["name"], i)
That would certainly make implementing your first suggestion a lot easier.
I can implement it now, if you think it's important enough. I suspect that
we won't see these much, but I dunno.
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
