Make sure you update copyrights to 2008.
Makefile:
- line 140: This doesn't seem to have been updated with the latest set of
python files. Please make sure to test Makefile installation on
Solaris.
file.py:
- line 176: I don't want for the try and the import mechanisms to get run
every time we come through here. Perhaps the import can be done
module-level, as before, but with in a try block where an ImportError
can simply set a boolean that's tested here. Would that work?
I'm also not sure that it's an error not to be able to check the elf
hash; it'll just make your downloads less efficient.
generic.py:
- line 139: 'closer' seems to actually be called 'cleanup'.
image.py:
- line 146: no need to wrap
elfextract.c:
- you've got a handful of blank lines you added tabs to. You've done
this (or adding spaces to blank lines) in a bunch of other places, too.
- line 377: why not the same sort of ifdef for verneed that you had for
verdef?
server/transaction.py:
- line 220: same on the try/import here as in file.py.
- line 226: I think you should have a more specific message here ("ELF
support not available" or something).
portable/__init.py__:
- line 240/241: Did you try doing something like:
from os_<implname> import *
That way you could avoid the double function call thing you've got
going on here. (I realize you'd have to define the class methods at
the top-level, too. And those that can be probably should be decorated
with @staticmethod.)
I think you'd also be better off making the various class instances
singletons, so that caches are shared process-wide. Or can you confirm
that's already the case?
- line 243: unused variable "e".
portable/os_unix.py:
- line 76 (et al): don't you want to catch KeyError here?
- line 128: "if passwd_stamp <= self.users_lastupdate.get(dir, 0):".
Note that you need to put "self." before all the "users_lastupdate"
mentions. Indeed, the code can't possibly work as-is.
portable/util.py:
- please use eight-space indents. (here and other places)
scripts/*.sh:
- what are these for?
api-complete.py:
- Why did you pull the platform, etc, computation out of the "main" area?
harness_lib.ksh:
- Vile indentation.
multiplatform.py:
- Is this used by pylint somehow? If so, should / can it be mentioned in
pylintrc?
I haven't reviewed setup.py or pylintrc in any detail. I'd like for Dan to
give his okay on the test bits before this comes back.
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
