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

snyk: do not throw away Snyk results by mistake #133

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

kdudka
Copy link
Member

@kdudka kdudka commented Oct 19, 2023

... on successful scans. This commit fixes a major regression introduced by the previous commit.

Fixes: commit e886342
Related: https://issues.redhat.com/browse/OSH-329
Resolves: https://issues.redhat.com/browse/OSH-362

@kdudka kdudka self-assigned this Oct 19, 2023
Copy link
Collaborator

@jperezdealgaba jperezdealgaba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

... on successful scans.  This commit fixes a major regression
introduced by the previous commit.

Fixes: commit e886342
Related: https://issues.redhat.com/browse/OSH-329
Resolves: https://issues.redhat.com/browse/OSH-362
Closes: csutils#133
@kdudka kdudka closed this in e1a73c6 Oct 19, 2023
@kdudka kdudka merged commit e1a73c6 into csutils:main Oct 19, 2023
2 of 41 checks passed
@lzaoral
Copy link
Member

lzaoral commented Oct 19, 2023

@kdudka @jperezdealgaba Thank you both for the quick fix!

It may be a good idea to extend the changes done in #95 with a follow up PR to also test the Snyk csmock plug-in so that we can prevent such breakages in the future.

Also, @jperezdealgaba, please, make sure to test your contributions properly. I'm sure that you have tested that the original PR fixed the crash reproducer but it is a very good practice to also check that the basic common use cases won't be affected by your changes and that they still work and produce reasonable results. Thank you!

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

Successfully merging this pull request may close these issues.

3 participants