Hey Keith,
This looks really good. Just a couple of comments:
usr/src/lib/install_common/__init__.py
--------------------------------------
Only a nit please check your blank lines for spaces.
usr/src/lib/install_common/test/test_install_popen.py
-----------------------------------------------------
It seems that the comment should appear before the assertFalse() or
that there is something else missing.
128 self.assertFalse(100 in logger.msgs, logger.msgs.get(100, ''))
129 # No output logged to the MockLogger
Anther nit is that the file is not pep8 clean, blank lines with spaces,
operators without spaces around them, blank line at end of file, ...
Thanks,
John
On 02/14/11 05:50 PM, Keith Mitchell wrote:
Hi all,
Can I get a review of my changes for these bugs please?
Bugs:
7019067 <http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7019067>
solaris_install.Popen logs blank lines
7018063 <http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7018063>
solaris_install.Popen should allow a user to ignore the check_code check
7018060 <http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7018060>
solaris_install.Popen should offer a DEVNULL destination
Webrev:
http://cr.opensolaris.org/~kemitche/webrev.7019067
In addition to resolving the listed bugs, I've:
- Added usage examples to the docstring for Popen
- Added docstrings to the test cases
- Resolved a bug where subprocesses that completed quickly may
hang in the select.select() call when attempting to perform logging.
(e.g., running Popen(["pkg", "foo"], logger=mylogger) would hit this)
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
