action #23546
closed[sle][functional] Improve sle_version_at_least, e.g. to consider CaasP versions, behave comparable to leap_version_at_least, etc.
0%
Description
Comment from PR (https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/3451):
@rwx788 like I said..it's important we get CaaSP honoring the sle_version_at_least
CaaSP 1.0 was mostly 12-SP2 with some 12-SP3, so the behaviour of at_least 12-SP2 would likely have worked
CaasP 2.0 will be 12-SP3 - we need it using the sle_version_at_least 12-SP3 behaviour
CaaSP 4.0 will be 15, which is why I think this is_sle approach is not acceptable - it will just break again in 5 months from now.
CaaSP is a sle distri, so we need to treat it as such
Problem (or confusing part is) sle_version_at_least returns true if VERSION value is anything which doesn't match SLE version naming convention. This is used in many places in TW, so we should approach it and make it more clear. E.g. leap_version_at_least returns false in case of non leap distri. This is inconsistent.
Updated by okurz over 6 years ago
- Subject changed from [sle][functional] Improve sle_version_at_least to consider CaasP versions to [sle][functional] Improve sle_version_at_least, e.g. to consider CaasP versions, behave comparable to leap_version_at_least, etc.
Updated by mkravec about 6 years ago
I don't think you need to consider CaaSP here, we use separate is_caasp sub.
Updated by riafarov about 6 years ago
@mkravec That's the problem because when we call sle_version_at_least we have to additionally call is_sle or is_caasp, which is confusing as method returns true for other distris. And that caused couple of regressions as well, as if you read function name, you don't expect that it will return true for openSUSE, for example.
Updated by mkravec about 6 years ago
I totally agree here - it's confusing and I also don't like it. Just wanted to say that you don't need to fix that for CaaSP.
sle_version_at_least is used ~200 times, so it will be donkey work to change that :)
Updated by mkravec about 6 years ago
@rwx788 - do you think we can close this? Now that we have following version_utils functionality I don't see what else can we do here.
is_sle '>=15'
is_caasp '>=4.0'
Updated by okurz about 6 years ago
I guess what we should do is get rid of all occurences of sle_version_at_least and leap_version_at_least using the new, nicer version functions you provided.
Updated by riafarov about 6 years ago
- Status changed from New to Resolved
@okurz, I agree with your statement, but I guess it's fine to handle it in another subticket. As stated above, we cannot simply remove all occurrences. So I would create a new ticket for that with more specific details and resolve this one, as we have more or less an idea only. Feel free to reopen and convert it to epic.
@mkravec, as stated above, let's now do proper review to avoid new usages and request changes to old code when related part is touched.