canonical / hotsos

Software analysis toolkit. Define checks in high-level language and leverage library to perform analysis of common Cloud applications.
Apache License 2.0
32 stars 38 forks source link

ycheck/requires/types/apt: decouple version range utilities from APT #852

Closed mustafakemalgilor closed 4 months ago

mustafakemalgilor commented 5 months ago

The version range functions 'normalize_version_criteria' and 'is_version_within_ranges' are not exclusive to apt and could be used by other classes. This patch moves these two functions to host_helpers/packaging/DPKGVersion, along with the unit tests. Added new unit tests for requires/types/apt. Reflected the changes to requires/types/binary and its unit tests.

Added repr class member fn to DPKGVersion to get a readable value when print() ing the DPKGVersion objects.

package_version_within_ranges is now called is_version_with in_ranges and it accepts a version instead of a package name.

BinCheckItems no longer inherits from APTCheckItems.

dosaboy commented 5 months ago

@mustafakemalgilor thanks for this. Before i get fully into the review, because the diff makes it quite hard to see what changed vs. what was moved can you tell me to what extent this is a lift and shift of the code and how different the new normalize_version_criteria() is from the old one?

mustafakemalgilor commented 5 months ago

@mustafakemalgilor thanks for this. Before i get fully into the review, because the diff makes it quite hard to see what changed vs. what was moved can you tell me to what extent this is a list and shift of the code and how different the new normalize_version_criteria() is from the old one?

Sure.

normalize_version_criteria is a verbatim copy, except it's now a static function instead of a class member one.

package_version_within_ranges:

mustafakemalgilor commented 5 months ago

@mustafakemalgilor thanks for this. Before i get fully into the review, because the diff makes it quite hard to see what changed vs. what was moved can you tell me to what extent this is a lift and shift of the code and how different the new normalize_version_criteria() is from the old one?

@dosaboy btw you can checkout this branch on your local and use git diff --color-moved HEAD~1 to get a diff that shows the changed/deleted/moved parts in a different color.

dosaboy commented 4 months ago

@mustafakemalgilor lgtm but can you please rebase because I think this might fail flake8 with changes that are now on master.

mustafakemalgilor commented 4 months ago

@mustafakemalgilor lgtm but can you please rebase because I think this might fail flake8 with changes that are now on master.

ofc, will do.