Shawn,

> http://cr.opensolaris.org/~johansen/pkg-timeout2/src/modules/client/image.py.wdiff.html
> ==========
>      1191 +                                        if len(fields) < 4:
> 
> A comment here explaining why you there is a varying number of fields
> would be nice.

I didn't actually change that code.  Take a look at the Sdiff instead.
I just wrapped the res.read() in a try/except block.  This seems to have
confused some of the diff tools, perhaps because the indentation changed.

> http://cr.opensolaris.org/~johansen/pkg-timeout2/src/modules/client/pkgplan.py.html
> ==========
>  234                         if dest:
>  235                                 dest.preinstall(self, src)
> ...
>  239                         if dest and dest.needsdata(src):
>  240                                 flist.add_action(dest)
> 
> Would that be better as:
> 
> if dest:
>         dest.preinstall(self, src)
>         if dest.needsdata(src):
>                 flist.add_action(dest)

I think it's safe to do this.

> http://cr.opensolaris.org/~johansen/pkg-timeout2/src/client.py.html
> ==========
> 1372         misc.MAX_TIMEOUT_COUNT =
> int(os.environ.get("PKG_TIMEOUT_MAX", "3"))
> 
> Doesn't the "hard-coded" 3 here make specifying a default value in
> misc.py not as useful?

Yes.  I'll fix this.

Thanks,

-j

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to