-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
use subprocess.DEVNULL instead of open(os.devnull) to avoid leaking a… #13329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## trunk #13329 +/- ##
=======================================
Coverage 58.04% 58.05%
=======================================
Files 88 88
Lines 5337 5338 +1
Branches 224 224
=======================================
+ Hits 3098 3099 +1
Misses 2015 2015
Partials 224 224 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please make the linter happy and I can merge this:
import os
import time
from platform import system
-from subprocess import STDOUT, DEVNULL
+from subprocess import DEVNULL
+from subprocess import STDOUT
from subprocess import Popen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @graingert!
… file handle
fixes #13328
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Description
Motivation and Context
Types of changes
Checklist