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

Add tests to see if maven/semver version_matches is right or not. #818

Merged
merged 5 commits into from
Sep 17, 2024

Conversation

bobmcwhirter
Copy link
Contributor

Seems to be right.

Copy link
Contributor

@jcrossley3 jcrossley3 left a comment

Choose a reason for hiding this comment

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

Is migration/ the right place for these tests to live?

with wackadoodle CVEs claiming Maven is semver, along
with our wackadoodle elision of actually checking version ranges.

And fix all of the above.
@@ -0,0 +1,110 @@

create or replace function mavenver_cmp(left_p text, right_p text)
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we have to do this logic in the db? Too much potentially returned to post-process? All the SQL makes my eyes bleed on to the vomit I just threw up. 👀 🍴 🤮

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, we want to do it in the DB, so that we can also support suitable pagination.

Else, we have to bring them all back, and scrobble around on the rust side winnowing things out of the version ranges.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add just a touch of Sriracha, and the bloodvomit will be lovely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was once in a band named "bloodvomit"

Copy link
Contributor

@jcrossley3 jcrossley3 left a comment

Choose a reason for hiding this comment

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

Thanks for the quick turnaround!

Comment on lines +318 to +321
/*
TODO: Gilles!
These statuses are *not* reflected in the CSAF regarding the quarkus 3.2.11 SBOM, which is the `sbom_id` from results[1] above.

Copy link
Contributor

Choose a reason for hiding this comment

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

Create an issue for this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

sounds good

@jcrossley3 jcrossley3 added this pull request to the merge queue Sep 17, 2024
Merged via the queue into trustification:main with commit 556482f Sep 17, 2024
2 checks passed
@bobmcwhirter
Copy link
Contributor Author

Thanks for merging! Your report, your privilege.

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