On Wed, Oct 01, 2008 at 04:34:43PM -0700, Dan Price wrote:
> On Wed 01 Oct 2008 at 04:18PM, Brad Hall wrote:
> > On Wed, Oct 01, 2008 at 05:35:42PM -0500, Shawn Walker wrote:
> > > Brad Hall wrote:
> > > >On Wed, Oct 01, 2008 at 03:25:09PM -0700, Dan Price wrote:
> > > >>On Wed 01 Oct 2008 at 03:16PM, Danek Duvall wrote:
> > > >>>On Tue, Sep 30, 2008 at 04:23:11PM -0700, Brad Hall wrote:
> > > >>>
> > > >>>>Ok, new webrev: http://cr.opensolaris.org/~bhall/bug-3588-2/
> > > >>>Looks fine to me.
> > > >>Since there's been some change in some of the areas you are touching,
> > > >>could you merge up and repost?
> > > >
> > > >Sorry, I thought I had done that already.. guess not.  Here is the merged
> > > >webrev: http://cr.opensolaris.org/~bhall/bug-3588-3/
> > > 
> > > client.py:
> > >   line 1981:  _("WARNING: there is already an image at: %s" % 
> > > image_dir)) -- you don't want the path to be passed to _(), so you 
> > > probably need to move the next to last ')' right after the ".
> > > 
> > >   lines 1978, 1980: since 'found' is only used on line 1978, why not 
> > > move the comparison to line 1980 and drop the un-needed variable?
> > > 
> > > Otherwise, looks fine.
> > > 
> > > Cheers,
> > > -- 
> > > Shawn Walker
> > 
> > Good points, here's a new webrev with those changes:
> > 
> > http://cr.opensolaris.org/~bhall/bug-3588-4/
> 
> I might be mistaken but I think that t_image_create.py:63 could
> be more concisely coded to use the exit=1 syntax we have in the test
> suite.
> 
> Why are we always forcing in the test suite?  Just curious?

Not doing that anymore.
 
> client.py:2013 I would suggest the slightly more concise
>     "To override, use the -f (force) option."
> 
> Also, 2012 isn't a warning: it's an error.  In the case that the
> user specified --force it might be a warning.
> 
> So, I would suggest:
> 
> # pkg image-create ... /tmp/foo
> pkg: there is already an image at: /tmp/foo
> To override, use the -f (force) option.

Changed.
 
> A related question: if the specified directory exists, but has other
> non-image junk in it, what should we do?

Sounds like we should error in that case, too.

New webrev up addressing these points and sync'd with the gate:
http://cr.opensolaris.org/~bhall/bug-3588-5/

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

Reply via email to