On Thu, Apr 10, 2008 at 01:25:47AM -0700, Dan Price wrote:
> http://cr.opensolaris.org/~dp/ips-perf
__init__.py
- line 156: why the " " + hash bit?
generic.py
- line 177: is there anything about orderdict that's instance-specific?
Couldn't we save a teeny bit of time and space by making it specific
either to the Action class or even the pkg.actions module?
- line 186: could be an elif (which means you can ditch the return on the
previous line)
- line 198: The else clause was there for more than just the None case --
it allowed you to store any sort of object in .data, which could later
be retrieved. We don't use it, so I suppose it's not critical, but
it'd be nice to keep it, and I don't think it hurts.
- line 233: inconsistent use of single-quotes vs backslash-escaping.
Would it be any slower to use %-expandos -- it'd likely be easier to
read.
filelist.py
- line 205: to stderr? Though this low-level code probably shouldn't be
printing at all ...
image.py
- line 896, 902: I'd probably rename gen_installed_pkgs() to
installed_pkgs(), and gen_installed_pkgs_forreal() to
gen_installed_pkgs(). That means renaming all the clients, but I think
it's a better naming scheme. We can postpone that if you like.
manifest.py
- line 257: I guess I was thinking that the actions should be agnostic in
regards to the information they carried, while the manifest was more
part of the system, where the idea that paths shouldn't start with the
root filesystem was more relevant, and that the "path" attribute was
universal across actions that represented filesystem objects.
- line 267: won't this have problems if self.size isn't set first?
- line 313: not using e.
- line 299: do we need this method any more?
- line 315: what if this fails?
- line 319: "packing"?
- line 329: You could split the open() into its own try block; then if
that succeeds but the dump fails, you could remove the file.
actionbench.py:
- line 53: ?
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
