Require valid_from in validity consistency check#631
Conversation
antonkri
requested review from
AlexanderLanin,
MaximilianSoerenPollak,
dcalavrezo-qorix and
nradakovic
as code owners
License Check Results🚀 The license check job ran with the Bazel command: bazel run --lockfile_mode=error //src:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
| return (major, minor, patch) | ||
|
|
||
|
|
||
| def _normalize_validity_attr(value: object | None) -> str | None: |
There was a problem hiding this comment.
I would prefer if this would be a generic function (name) instead of one specialized for validity attributes.
For the code itself, is there a scenario where the value is NOT an int/float or a string?
Like could it be a list or so?
If that is the case we would return just the list as a string here, not sure that is what we want?
So we probably should program more defensively to error in such scenarios too.
| return | ||
|
|
||
| if not valid_from or not valid_until: | ||
| if not valid_until: |
There was a problem hiding this comment.
There is no error msg here, but there is one in valid_from? Is this an oversight or on purpose?
antonkri
force-pushed
the
ankr_check_reqs_valid_from
branch
2 times, most recently
from
c066ec5 to
cb37328
Compare
|
|
||
| if not valid_from or not valid_until: | ||
| if valid_until_values and not valid_from_values: | ||
| log.warning_for_need(need, "is missing required attribute: `valid_from`.") |
There was a problem hiding this comment.
A bit confused here.
According to metamodel.yaml valid_from and valid_until are both optional options.
Here now one is protrayed as mandatory option.
Did you forget to add the change to the metamodel.yaml?
There was a problem hiding this comment.
Hey Maximilian, no. What I want to achieve is that everyone do the scoping of their requirements and tell us, which requirements are valid for release score v1.0 and which for later releases. In my opinion, valid_until is not mandatory, as the requirement can be valid forever. But valid_from is more important as we need to know, when the implementation is available for the first time and can be e.g. tested.
There was a problem hiding this comment.
...sorry The metamodel was changes in the previous commit, please check
There was a problem hiding this comment.
Please set the flags to mandatory in the metamodel and add the checks as implemented in tool requirements
| log.warning_for_need(need, "is missing required attribute: `valid_from`.") | ||
| return | ||
|
|
||
| if not valid_from_values or not valid_until_values: |
There was a problem hiding this comment.
This is still an issue I think, as valid_from_values is mandatory but valid_until_values is not.
Therefore if valid_until_values is not set this will still enter and exit early
antonkri
force-pushed
the
ankr_check_reqs_valid_from
branch
2 times, most recently
from
4a4c3d1 to
2a5ba91
Compare
antonkri
force-pushed
the
ankr_check_reqs_valid_from
branch
from
2a5ba91 to
e11edfe
Compare
3 participants
Require and normalize valid_from in validity consistency check