[issue-403] change spdx version handling during validation #404
Conversation
armintaenzertng
force-pushed
the
refactor-python-tools
branch
from
48c4e72
to
5eb63e4
Compare
armintaenzertng
force-pushed
the
validationVersionFromDoc
branch
from
23d7208
to
f78e2af
Compare
src/validation/document_validator.py
Outdated
| validation_messages: List[ValidationMessage] = [] | ||
|
|
||
| validation_messages.extend(validate_creation_info(document.creation_info)) | ||
| if not spdx_version: | ||
| spdx_version = document.creation_info.spdx_version |
There was a problem hiding this comment.
instead of setting the version here, I think it would be cleaner to make the version optional in the validation_messages.extend(validate_creation_info( call and only assert on it, if it is set.
There was a problem hiding this comment.
I agree that it should be optional in validate_creation_info(), but we have to set the version here, nevertheless, as it is a mandatory field for other validations like validate_relationships() (and more to come later).
There was a problem hiding this comment.
you could still move the spdx version validation up, to have it cleaner
There was a problem hiding this comment.
Its just a minor improvement, but:
| spdx_version = document.creation_info.spdx_version | |
| validation_messages.extend(validate_creation_info(document.creation_info, spdx_version)) | |
| if not spdx_version: | |
| spdx_version = document.creation_info.spdx_version |
There was a problem hiding this comment.
I have extracted the version validation to be handled before everything else, now.
Signed-off-by: Armin Tänzer <armin.taenzer@tngtech.com>
Signed-off-by: Armin Tänzer <armin.taenzer@tngtech.com>
armintaenzertng
force-pushed
the
validationVersionFromDoc
branch
from
f78e2af
to
c3f78a1
Compare
fixes #403
Signed-off-by: Armin Tänzer armin.taenzer@tngtech.com