-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide more information in get_components_without_* functions #169
Conversation
Optionally return both the name and the SPDXID of problemative elements in get_components_without_* functions Signed-off-by: Gergely Csatari <gergely.csatari@nokia.com>
One thing what I could not decide if should all the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this PR. Two small nits: the CI is failing because of a formatting issue and a lint issue. Can you please fix those? black
is the formatter this project uses. pylint
is the linter this project uses.
See my questions in the original issue for a more substantive discussion. Regardless though, this is a nice and clear PR. Thank you!
I look forward to discussing this change, or a very similar change, more.
tests/test_checker.py
Outdated
# TODO: Not sure how to test this. If any package misses the SPDXID the whole file seems to be invalid. | ||
#components = sbom.get_components_without_identifiers() | ||
#assert components == ["glibc-no-identifier"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is un-testable, I also think, given the constraints of the spdx-tools
python package that powers this project. I've run into this "problem" too. Since I think that's the case, it's probably just worth explaining this situation in the comment directly to help aid future developers.
@@ -74,16 +74,19 @@ def get_components_without_names(self): | |||
components_without_names.append(package.spdx_id) | |||
return components_without_names | |||
|
|||
def get_components_without_versions(self): | |||
"""Retrieve SPDX ID of components without names.""" | |||
def get_components_without_versions(self, returnTuples=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my question in the original issue about whether the option should be returnTuples
or something more like returnSDPXIDs
. If you think returnTuples
is indeed the right way to go, then this code looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my use case the tuples would be preferable. I added a longer explanation to here
Signed-off-by: Gergely Csatari <gergely.csatari@nokia.com>
I've fixed these in dc3c53d. Maybe |
Yes, definitely. Good call. Issue created: #170 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thank you, @CsatariGergely!
@goneall: Can you please take a look too when you have a moment? Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - I like the approach of the optional parameter, should keep things compatible
Optionally return both the name and the SPDXID of problematic elements in get_components_without_* functions.
Closes #168