On 04/18/11 06:02 PM, Brock Pytlik wrote:
On 04/18/11 05:14 PM, Shawn Walker wrote:
On 04/18/11 04:39 PM, Brock Pytlik wrote:
On 03/11/11 03:41 PM, Shawn Walker wrote:
Greetings,
The following changes implement a user interface for removing packages
(the support for removing them has been implemented since build 128 or
so, but no user interface existed until now):
7213 pkgrepo should support package removal
webrev:
http://cr.opensolaris.org/~swalker/pkg-remove/
...
catalog.py:
line 45: why this change? It just seems to add noise. Using pfmri is the
convention we've adopted elsewhere in the code and I'd like to continue
that here.
Because my general preference is import modules without using aliases.
I also don't like using variable names the same as modules I'm
importing. This just seems safer.
That may be, but it conflicts with the other imports in this module and
the imports of pkg.fmri in other modules. Please don't introduce this
incompatibility. If you think we're doing it wrong, file a bug and let's
change everywhere in once big swatch if we, as a team, agree that your
preferences are better than how we've been doing it in the past.
It's not "incompatible". It's just different. And we're not even
consistent in this regard; look at other modules.
line 2782: I think this comment should either be expanded upon or
removed.
Not much to expound on; not my original comment.
I don't understand, this all looked like new code to me. If this isn't
your comment, who's is it? Or more to the point, why is it there? What
information is it trying to convey?
I've updated it actually in the webrev.
...
repository.py:
line 1553: I think it's worth breaking out of the loop if there aren't
any pfiles remaining.
Best I can do without more changes to progress tracking is to
short-circuit the loop with something like:
for ver in os.listdir(pdir):
if not pfiles:
progtrack.actions_add_progress()
continue
...so that progress tracking doesn't break.
Well, it seems to me we could change progress tracking in the situation
where we know we're intentionally breaking out of the loop.
That's a bigger project than I want to take on at the moment. But I
have made this better as I noted by changing it to short-circuit the loop.
...
I'm assuming you've tried using search and found it too slow? I would've
I cannot rely on search here, especially since there's no guarantee a
search database will be present. I've purposefully avoided relying on
search due to other issues we haven't yet resolved (such as removed
packages, etc.).
I was just suggesting it as an optimization. Seems a shame to waste a
file which, if present, associates files with packages.
As a future optimisation, it may be nice, at the moment, I think it's
best that it doesn't use search.
...
line 2576: shouldn't there be a error or warning if a repository doesn't
contain a catalog? I think the user may be surprised otherwise.
No; it's an expected condition.
line 2597: Why are we del'ing these here?
Since it's a potentially large set of data and they're not needed
after this point.
Really? The set of fmris is a large data set? Seems strange to me to del
them each loop since the next time through the loop, they get
overwritten anyway, but whatever.
Yes, possibly. And more annoyingly, after the loop finishes execution,
Python will still leave them in scope with all of their data hanging
around until the function finishes execution.
...
pkgrepo.py:
We're ok as a group as making -s a required argument? I guess I was
surprised that wasn't a separate bug.
In this particular case, the usage was just wrong. I'm not changing
behaviours. pkgrepo has never allowed the use of environment variables
for specifying destination repository, so -s is the only way to
specify one. I can file a trivial bug if necessary.
Ok, the changes in the usage made me think otherwise.
Yes, it could be misleading. I'll add a bug to the synopsis just for
clarity.
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
