SP-3561_conversion-issues #156
Conversation
WalkthroughUpdated package to v1.37.1; added license-source filtering for CycloneDX parsing, relaxed SPDX license-source acceptance for unspecified sources, and adjusted CycloneDX produce_from_str to unpack produce_from_json's tuple. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant CycloneDX
participant Producer as produce_from_json
Caller->>CycloneDX: call parse(xml/json)
CycloneDX->>CycloneDX: extract licenses
note right of CycloneDX #E6F4EA: keep only licenses with\nsource in {component_declared, license_file, file_header}
CycloneDX-->>Caller: parsed component data
Caller->>CycloneDX: produce_from_str(input)
CycloneDX->>Producer: produce_from_json(parsed)
Producer-->>CycloneDX: (success, data)
note right of CycloneDX #FFF7E6: unpack tuple and\nreturn success flag
CycloneDX-->>Caller: success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
CHANGELOG.md (1)
698-698: Optional: add an Unreleased comparison link for completeness.Add a compare link for the Unreleased section to follow the existing pattern.
[1.37.0]: https://github.com/scanoss/scanoss.py/compare/v1.36.0...v1.37.0 [1.37.1]: https://github.com/scanoss/scanoss.py/compare/v1.37.0...v1.37.1 +[Unreleased]: https://github.com/scanoss/scanoss.py/compare/v1.37.1...HEADsrc/scanoss/spdxlite.py (1)
228-239: Align docstring with implemented policy (include file_header, clarify None/empty).Update the comment to reflect accepted sources and the fact that None/'' are permitted (i.e., only non-empty, non-allowed sources are filtered).
- This method filters license information to include only licenses from trusted sources - ('component_declared' or 'license_file') and removes any duplicate license names. + This method filters license information to include only licenses from trusted sources + ('component_declared', 'license_file', 'file_header'). Licenses with an unspecified + source (None or '') are allowed. Non-empty, non-allowed sources are excluded. It also + removes any duplicate license names.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.md(2 hunks)src/scanoss/__init__.py(1 hunks)src/scanoss/cyclonedx.py(2 hunks)src/scanoss/spdxlite.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/scanoss/cyclonedx.py (2)
src/scanoss/spdxlite.py (1)
produce_from_json(276-293)src/scanoss/csvoutput.py (1)
produce_from_json(182-222)
🔇 Additional comments (3)
CHANGELOG.md (1)
12-16: Changelog entry reads well and matches the code changes.Version/date and notes are consistent. Nothing else to change here.
src/scanoss/__init__.py (1)
25-25: Version bump matches CHANGELOG.All good.
src/scanoss/cyclonedx.py (1)
302-304: No changes required; return-type change is correct.Verification confirms all callers of
produce_from_streither expect a bool return or ignore the return value. No caller attempts tuple unpacking or dict access. The change is safe and introduces no breaking changes.
| name = lic.get('name') | ||
| source = lic.get('source') | ||
| if source not in ('component_declared', 'license_file', 'file_header'): | ||
| continue | ||
| fdl.append({'id': name}) | ||
| fd['licenses'] = fdl |
There was a problem hiding this comment.
License source filtering for file matches looks good; mirror this for dependencies.
You correctly keep only ('component_declared', 'license_file', 'file_header') here. The dependency path above (Lines 91–103) still accepts all sources and should apply the same filter for consistency.
Suggested change for the dependency block:
- licenses = deps.get('licenses')
- fdl = []
- if licenses:
- dc = []
- for lic in licenses:
- name = lic.get('name')
- if name not in dc: # Only save the license name once
- fdl.append({'id': name})
- dc.append(name)
+ licenses = deps.get('licenses')
+ fdl = []
+ if licenses:
+ seen = set()
+ for lic in licenses:
+ name = lic.get('name')
+ source = lic.get('source')
+ if source not in ('component_declared', 'license_file', 'file_header'):
+ continue
+ if name and name not in seen:
+ fdl.append({'id': name})
+ seen.add(name)
fd['licenses'] = fdlCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/scanoss/cyclonedx.py around lines 91 to 103, the dependency license
handling currently accepts all license sources while the file-level code (lines
155–160) restricts to ('component_declared', 'license_file', 'file_header');
update the dependency block to apply the same filter by checking
lic.get('source') and only appending licenses whose source is one of those three
values so dependency licenses mirror the file-level filtering logic.
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
| name = license_info.get('name') | ||
| source = license_info.get('source') | ||
| if source not in ("component_declared", "license_file", "file_header"): | ||
| if source not in (None, '') and source not in ("component_declared", "license_file", "file_header"): |
There was a problem hiding this comment.
This could be simplified to
if source not in (None, '', "component_declared", "license_file", "file_header"):
Summary by CodeRabbit
New Features
Bug Fixes
Chores