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

package_query: Fix filter_version with non EQ comparator #1127

Merged
merged 2 commits into from
Jan 16, 2024

Conversation

m-blaha
Copy link
Member

@m-blaha m-blaha commented Jan 4, 2024

The current implementation of the filter_version and filter_release filters
always uses the cmp_eq comparator. This means that these filters only work
with EQ and NEQ comparisons. The patch fixes these filters to work
correctly with LT, LTE, GT, and GTE comparisons as well.

The current implementation of the filter_version and filter_release
filters always uses the cmp_eq comparator. This means that these filters
only work with EQ and NEQ comparisons. The patch fixes these filters to
work correctly with LT, LTE, GT, and GTE comparisons as well.
Now the tests use also LT, LTE, GT, and GTE comparisons.
@j-mracek j-mracek self-assigned this Jan 4, 2024
@j-mracek
Copy link
Contributor

j-mracek commented Jan 4, 2024

LGTM, Thank you for the patch

@j-mracek
Copy link
Contributor

j-mracek commented Jan 5, 2024

I have to test it locally

@m-blaha
Copy link
Member Author

m-blaha commented Jan 5, 2024

Actually if you check testing farm results (e.g. here https://artifacts.dev.testing-farm.io/70790912-a7e2-41e2-8d9e-758da7d0ef10/) and switch on "show passed tests" on top of the page, you will see that only failing tests are dnf4 ones.
But I agree, it's PITA and we need to fix it ASAP.

@j-mracek
Copy link
Contributor

LGTM

@j-mracek j-mracek added this pull request to the merge queue Jan 16, 2024
Merged via the queue into main with commit d745092 Jan 16, 2024
5 of 9 checks passed
@j-mracek j-mracek deleted the mblaha/filter-version branch January 16, 2024 11:32
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.

2 participants