Skip to content
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

Remove snyk as a blocking CI step #5327

Closed
kieckhafer opened this issue Jul 16, 2019 · 3 comments
Closed

Remove snyk as a blocking CI step #5327

kieckhafer opened this issue Jul 16, 2019 · 3 comments

Comments

@kieckhafer
Copy link
Member

Snyk frequently comes up as a blocking step in our CI process.

This doesn't fully make sense, especially since the majority of the time, what Snyk flags is already in develop branch of Reaction, and not introduced by the PR being flagged.

This causes unneeded frustration for developers, having PR's blocked by things they have no control over.

We should remove SNYK as a blocking CI step, and use it for reference to improve the app instead. We can also look into adding different apps that might be easier to work with: Automation via Zapier, If This Than That, etc.

@kieckhafer
Copy link
Member Author

@focusaurus @aldeed @bt3gl @spencern Does this seem like SNYK should only be running if package.json is different than the published branch? I think that's what we want, so maybe this isn't working? https://github.com/reactioncommerce/reaction/blob/ci-kieckhafer-addByChangedFileLinting/.circleci/config.yml#L339

@aldeed
Copy link
Contributor

aldeed commented Jul 24, 2019

@kieckhafer Yes I'm not sure if it's still using that check or if it's some integration on the Snyk side now. The best would be to test by changing some file (not package.json) in a fork to your personal GitHub, and PR that back.

That said, we had discussed this and thought in general that we should remove Snyk entirely from CI checks. The reason is because Snyk check results change randomly, depending on when security bugs are found and reported. So it makes more sense to run them as a scheduled job rather than triggered by CI. They are already scanned daily and results are sent to those who are subscribed on Snyk.

@kieckhafer
Copy link
Member Author

Closed via #5403

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants