On 01/31/11 01:29 PM, Drew Fisher wrote:
Trying this again since something caused the first one to bounce...
Keith,
__init__.py:
1 - do you need the shebang here?
Due to quirks in pkgdepend(1), yes, all Python files in our gate should
have a Python shebang line. (If you know of any that don't, that's a bug)
150 - change to: if logger is not None:
151 - remove this line?
Updated.
Other than that, this looks great. Where was this 7 months ago when
we did DC? :)
7 months ago I was still convinced that subprocess.Popen was "good
enough" for anything we had to do! Sorry!
- Keith
-Drew
On 1/31/11 1:20 PM, Keith Mitchell wrote:
Hi all,
I'd like a code review for my fixes for:
15957 <http://defect.opensolaris.org/bz/show_bug.cgi?id=15957> Implement
solaris_install.Popen
7014402
<http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7014402> SUNW*
packages showing up in install-redistributable
webrev:
http://cr.opensolaris.org/~kemitche/webrev.15957/
Notes:
* solaris_install.Popen is a drop-in replacement for subprocess.Popen
- as verification, the test code actually runs the full
subprocess.Popen test suite against solaris_install.Popen.
* "exec_cmd" and its family have not yet been removed. I will file
bugs against the various installers to replace usage of such
functions with solaris_install.Popen invocations; this allows for new
consumers to use solaris_install.Popen now, rather than waiting an
indefinite amount of time for me (or someone) to pull out those
functions and widely re-test a large portion of the slim_source codebase.
* I initially considered doing a global find/replace of
subprocess.Popen with solaris_install.Popen. However, since
solaris_install.Popen is in the solaris_install module, in many cases
that results in a new Python import of a module not in the Python
standard library. This has potential implications on packaging
dependencies; as such, there are risk and time factors involved in
such a find/replace (in the form of at least sanity checking most
installers). As there will be bugs to remove "exec_cmd" functions
anyway, it seems appropriate to have those bugs also replace
subprocess.Popen with solaris_install.Popen. (The current approach
also is probably easier for the various projects to merge with)
Thanks,
Keith
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
