Hide command prompt on Windows when running subprocess. #136
Conversation
svn/common_base.py
Outdated
| stderr=subprocess.STDOUT, | ||
| cwd=wd, | ||
| env=env) | ||
| if sys.platform.lower().startswith('win'): |
There was a problem hiding this comment.
subprocess.STARTF_USESHOWWINDOW is only defined in Windows. Just try to get it and handle AttributeError if it doesn't exist. We don't want to get involved with detecting platform names.
svn/common_base.py
Outdated
| startupinfo = subprocess.STARTUPINFO() | ||
| startupinfo.dwFlags |= subprocess.STARTF_USESHOWWINDOW | ||
|
|
||
| p = subprocess.Popen( |
There was a problem hiding this comment.
Just create an empty kwargs variable above, add startupinfo to it if we're in Windows, and expand/unsplat the kwargs into the existing call rather than duplicating it.
svn/common_base.py
Outdated
| startupinfo = subprocess.STARTUPINFO() | ||
| startupinfo.dwFlags |= subprocess.STARTF_USESHOWWINDOW | ||
|
|
||
| p = subprocess.Popen( |
There was a problem hiding this comment.
While we're here, let's refactor to use subprocess.check_output() rather than subprocess.Popen() directly.
There was a problem hiding this comment.
That could go in another pull request as just a code refactor.
|
Also, unit-test coverage dropped by a third with your change. Please investigate. |
|
I've updated the pull request. Unsure why unit-test coverage dropped by a third. It is either unrelated to this pull request or coveralls reporting is inaccurate. Other pull requests have similar issues. |
dsoprea
force-pushed
the
master
branch
2 times, most recently
from
544f7da
to
230a2cd
Compare
|
Closed due to ignored conflicts. |
No description provided.