Hey Danek,
Thanks for looking at this.

> Don't check in pkgrecv.1 -- just the .txt file is needed.  And wrap that
> long long line.

Ok.  I've done both.

> pkgsend.py:
> 
>   - I'd use -d instead of -b -- I think we'll want -b for prepending a path
>     onto the path attribute.  pkgmk uses -r for what you're using -b for,
>     if you want to do that instead of -d.

I'm not attached to -b.  I'll change this to -d instead.

>   - line 219: I'd refactor this a bit.  In particular, everything currently
>     in the else clause could be outside, with just fullpath being set
>     appropriately in the if and the else side.

Thanks, some of that was awfully redundant.  Changed.

> testutils.py:
> 
>   - line 342, 346: spaces around =.

Fixed.  That'll teach me to copy Dan's code.  :P

> t_recv.py:
> 
>   - line 85: this is probably a copy from somewhere else, but this does
>     mean that two people can't run tests on the machine at the same time.

Yeah, I copied this out of another test to save myself time.  I'm
assuming it's okay to leave this as is, since we'll have to clean a
bunch of other tests if we actually want to support multiple people
running cli-complete.py on the same machine at the same time.

>   - line 89: need parens.

That was also copied out of t_upgrade.py, just like the stuff above.
I've added the parens to that file too.

> pull.py:
> 
>   - did you think about putting the files in the same directory structure
>     as they are on the server?

No.  pkgsend include expects the files to be in the current working
directory.  I added the -b option (now -d) to pkgsend to the testsuite
could run in another directory and specify where all the files were
located.

I had initially set up the directory structure to include version
numbers, but Stephen didn't think that was necessary.

>   - line 77: why not just do the normal action parsing stuff?

The current stuff that parses the actions assumes that you're attached
to an image.  I looked at some of this initially, but I found that it
was going to be a lot simpler to write it this way.

We'll eventually need to refactor all of the code so pkgrecv can use the
interfaces without needing to create an image, but that seemed beyond
the scope for a first iteration.

I've put the changes into a new webrev available from:

http://cr.opensolaris.org/~johansen/webrev-pkgrecv2/

Thanks for looking at this!

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

Reply via email to