search

spdx / ntia-conformance-checker

Check SPDX SBOM for NTIA minimum elements
Apache License 2.0
47 stars 18 forks source link

Find the DESCRIBES relationship by looking through attached packages #189

Closed DanielOjalvo closed 1 week ago

jspeed-meyers commented 3 weeks ago

@DanielOjalvo, thank you! Can you please use black and pylint to fix the minor formatting issues?

You can get more information here.

jspeed-meyers commented 3 weeks ago

QUESTION: Is this a breaking change or not? The tests pass, but I worry changing the internal logic of a key function SHOULD be a breaking change. Hmmm. Thoughts from anyone?

jspeed-meyers commented 3 weeks ago

One other question, which was raised in the discussion in #186: Is the intent of this check actually to determine if there is at least one root node in an SPDX document? This question and its answer is probably out of scope for this PR and issue. To me, it sounds like this should be a discussion of how the SPDX NTIA mapping document defines this check. I worry that the written definition ("The document must DESCRIBES at least one package") and the intent I read in the related issue thread aren't the same, but maybe I am confused. (I am often confused!)

Anyways, I wanted to discuss this before merging this change.

jspeed-meyers commented 3 weeks ago

@goneall: Can you please review this PR too? For the actual code review, can you please take a narrow view, simply checking if the logic of the code matches the current written intent of the SPDX NTIA mapping? And for answering the other questions, can you please your broad SPDX expertise and advise? Thank you, as always.

goneall commented 3 weeks ago

cc: @kestewart - Kate - let me know if you agree / disagree with my above conclusion.

jspeed-meyers commented 3 weeks ago

Thank you, @goneall. I'll give Kate time to weigh in before merging. [EDITED MY COMMENT. I confused myself. Please ignore.]

jspeed-meyers commented 2 weeks ago

I'll merge on Friday, June 28, 2024 unless I hear any objections. I won't cut a new release (3.0.0 I suppose) immediately. I'll wait a month or so to see if there are any other bugs that are breaking changes that emerge.