-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
Run coverity on PRs #13117
Comments
If that’s feasible, I see no reason not to? I guess the reason to only send it to the security working group is that it might show up bugs relevant to security, but if we catch those problems even before they enter /cc @nodejs/build |
Does this step run by CTC on PR after reviewing or by contributors themselves? |
I did some digging through old emails. It looks like @rvagg and @jbergstroem set up the Coverity account. It also looks like Coverity is an external service that can take up to 48 hours to send back a report. Unless this has changed since 2015, we may not be able to run it on every PR, although I agree it would be nice to be able to. |
It seems like perhaps this should be closed (based on @cjihrig's comment above). Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that. |
AIUI we currently run coverity on master and send the results to the security group. Given that it seems to turn up useful things (most recently #13050 (comment)) we should probably run it on PRs as well.
Thoughts?
cc/ @sam-github @cjihrig
The text was updated successfully, but these errors were encountered: