On Wed, Sep 24, 2008 at 03:27:49PM -0700, Danek Duvall wrote:
> On Tue, Sep 23, 2008 at 03:36:51PM -0700, Brad Hall wrote:
> 
> > http://cr.opensolaris.org/~bhall/bug-388-2/
> 
> client.py:
> 
>   - line 324: no need for pkgerr; you can just test failed_actions.

Ok, fixed
 
> pkg.1.txt:
> 
>   - You no longer implement -n, so it should get removed from the docs.

Removed
 
> pkgplan.py:
> 
>   - line 120: If I'm reading this correctly, you're only going to be fixing
>     hardlinks that are in this package, rather than in any package on the
>     system.

Yes, that's true for now.  Maybe we can just add an XXX/file a bug for this?
It seems expensive to check every hardlink in every other package on the
system unless you know of a better way of doing it.
 
>   - line 127: why cmp() instead of ==?

Changed
 
> file.py:
> 
>   - line 53: I didn't respond to your question about my comment on this
>     before, but the problem is that this flag, as a class variable, is set
>     to be the same for each instance of the class.  So if you have a
>     hundred file actions which simply need their modes changed, and one
>     which needs to be downloaded again, they'll all be downloaded again.

Changed, does it look better now?
 
> hardlink.py:
> 
>   - line 101: I think the comma there needs to be a percent.

Oops, fixed.
 
>   - line 104: if either the path or the target doesn't exist, then the test
>     here will throw an exception.  Like you did in the file action, you
>     probably should return before this if there've been any errors.

Ok, fixed.

>   - line 109: Typo in "unexpected".

Fixed
 
> Bart also reminded me yesterday that you'll need to do the same thing with
> directories as upgrade does -- if a directory gets "fixed" back into a
> file, and has any contents, the directory will need to be salvaged.

OK, added similar code to FileAction::install().

Webrev updated in place.

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

Reply via email to