On Wed 05 Aug 2009 at 02:13PM, Bart Smaalders wrote:
> A rewrite of solaris.py....
>
> http://cr.opensolaris.org/~barts/importer/
>
> Fixes
>
> 3395 importer can't find dependencies hidden behind symlinks
> 9886 solaris.py needs to be smarter when dealing with duplicate editable
> files
> 10342 solaris.py does not pass import failures to final exit status
>
> Does not yet address
>
> 10476 importer should be capable of resolving package dependencies
> against repo
>
> although that's pretty close once we decide what to do about transport
> (new transport code is image-centric, which doesn't apply here).
Are you going to retire solaris.py too? I could see other people
playing in this space getting tripped up if they continue to use it.
Since I wasn't too familiar with the old code, I wasn't sure
which of my comments below pertained to what you changed vs.
what was copied in from solaris.py...
---
47: I would explain this utility in some other way than "rewrite of
solaris.py" :)
---
48: chatters -> chattr's?
---
52: package -> Package? I thought the convention was to capitalize
classnames.
---
I ran this through our pylint-- it found a bunch of things. You
might consider cleaning those up? I realize that some of this code is
assert "not implemented", but that seems like sort of a bug too,
since that assert won't actually fire:
>>> assert "Not implemented"
>>>
Maybe:
assert "Not implemented" == 0
Or:
raise NotImplementedError("not done yet")
---
Also, I would recommend putting what is essentially "main" into a function, and
calling that. This helps a lot with pylint because it reduces the number of
variables which are at global scope.
---
A number of end-of-line comments seem to stray over 80 chars, and
could be trivially fixed:
p = (l[2:].split()[0]) # first part of string is path (r
emoves options)
---
In the svr4_pkg_list code, when sending data to the server, perhaps stat() the
file you're sending, and ensure that it is, indeed, the same as
attrs["pkg.size"]? Or is it impossible to tell in this context?
Will we get an EOF?
---
154: I had thought we had agreed to stop publishing opensolaris.zone
in favor of the variant at some point soon? I can't remember.
--
903-922: sys.exit(2) is traditional for invocation issues.
---
1005: Summary comment or docstring?
---
1176: Put crosschecks into a subroutine?
---
1188, 1207, 1254: sys.exit(1) (or something other than 2) is traditional for
errors
---
-dp
--
Daniel Price, Solaris Kernel Engineering http://blogs.sun.com/dp
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
