I updated this webrev to capture some additional comments from Danek.
This also fixes some bugs found with and contained within the test
cases.  It's still located in the same place.

-j

On Fri, May 02, 2008 at 07:05:13PM -0700, [EMAIL PROTECTED] wrote:
> 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