feat: treat version as int#599
Conversation
AlexanderLanin
requested review from
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 |
MaximilianSoerenPollak
left a comment
MaximilianSoerenPollak
left a comment
There was a problem hiding this comment.
Just some small simplification.
But overall this looks good and we can merge it.
All things mentioned are nitpicks and can be done in future PR's
| raw_value = need.get(key, None) | ||
| if not raw_value: | ||
| return [] | ||
| if isinstance(raw_value, str): |
There was a problem hiding this comment.
Could you not do:
| if isinstance(raw_value, str | int): |
to check both?
There was a problem hiding this comment.
yes and no. I would keep it separate as casting int to a str feels like a dirty temporary workaround. Hence the comment.
| for item in raw_list: | ||
| if not isinstance(item, str): |
There was a problem hiding this comment.
| for item in raw_list: | |
| if not isinstance(item, str): | |
| if not all(isinstance(item, str) for item in raw_list) |
Making it shorter / more efficent with list comprehension?
| bazel_dep(name = "score_process", version = "1.5.4") | ||
| git_override( | ||
| module_name = "score_process", | ||
| commit = "1c7867b85a1c1ca915bd516cb60ef2e9c1904975", |
There was a problem hiding this comment.
Probably should now adapt this to the latest commit that was just merged.
There was a problem hiding this comment.
used latest commit hash. requested process 1.6.0 for later today
2 participants
Warning: this is based on unreleased process_description
Note: consumer test on score module fails, as that pulls last released process description.