Tim Foster wrote:
Hi Shawn,

On Fri, 2010-01-22 at 11:17 -0600, Shawn Walker wrote:
line 2368, 2370: instead of "if <var> and len(<var>) > 0" just use "if <var> is not None" as everything else around it does

You're right, I misread the comment,
--
2353 # None is checked for here so that a client can unset a ssl_cert or
2354 # ssl_key by using -k "" or -c "".

thinking that ssl_cert was being set to '""' - sorry.

I didn't see this change in the updated webrev. Forget to regenerate or rsync? I don't know if we really care or not, but I think we've generally been doing os.path.join(a,b) instead of a + os.sep + b.
line 3205: s/os.getcwd()/orig_cwd/ ?

Sure - I've moved the try/except blocks from there up to where we
define orig_cwd.
3132-3137: I think if you change 3133 to orig_cwd = os.environ.get("PWD", None)
you drop the enclosing try/except block.

Other than that, things LGTM.
Brock
I'll assume you've tested this locally (I know it's nearly impossible to test SSL stuff in the test suite at the moment).

Yes, tested on a local image here against the extras repository with
some valid keys/certs using valid absolute and relative paths to them
ensuring the correct path gets written to cfg_cache, and
invalid absolute and relative paths.  I've also run the test suite just
to be sure the extra changes in client.py don't break anything else.

I've updated the webrev at
http://cr.opensolaris.org/~timf/11554,13987-pkg-arg-checks

Thanks for the review,
                
        cheers,
                        tim




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

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

Reply via email to