Keep non-FSF copyrights in GPL license text files - #303
Conversation
📝 WalkthroughWalkthroughThe change adds a shared helper that removes copyright entries containing “free software foundation” from GPL-family license text. Both ScanCode parser paths use the helper and retain other copyright entries. Tests cover GPL-family and non-applicable inputs. ChangesGPL copyright filtering
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/fosslight_source/_parsing_scancode_file_item.py (1)
233-235: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd regression coverage for both parser paths.
The supplied test at
tests/test_tox.py:113-137verifies only the GPL-family gate. Add cases for mixed-caseFree Software Foundationtext and another copyright entry. Assert that bothparsing_scancode_32_earlierandparsing_scancode_32_laterremove only the FSF entry.Also applies to: 730-732
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_source/_parsing_scancode_file_item.py` around lines 233 - 235, Add regression tests covering both parsing_scancode_32_earlier and parsing_scancode_32_later with mixed-case “Free Software Foundation” text plus another copyright entry; assert each parser removes only the FSF entry while preserving the other entry, without changing the existing GPL-family coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/fosslight_source/_parsing_scancode_file_item.py`:
- Around line 63-69: In exclude_free_software_foundation_copyrights, rename the
list-comprehension variable copyright to a non-shadowing name such as
copyright_entry and update its references within the comprehension, preserving
the existing filtering behavior.
---
Nitpick comments:
In `@src/fosslight_source/_parsing_scancode_file_item.py`:
- Around line 233-235: Add regression tests covering both
parsing_scancode_32_earlier and parsing_scancode_32_later with mixed-case “Free
Software Foundation” text plus another copyright entry; assert each parser
removes only the FSF entry while preserving the other entry, without changing
the existing GPL-family coverage.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: df5652c6-45ea-41ae-b6b8-436d62bd049c
📒 Files selected for processing (1)
src/fosslight_source/_parsing_scancode_file_item.py
Signed-off-by: Park Wonjae <wonjae.park@lge.com>
8149292 to
524b693
Compare
Collapse GPL-family detection and FSF copyright exclusion into a single filter so call sites no longer need separate predicates and branches.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/fosslight_source/_parsing_scancode_file_item.py`:
- Around line 216-218: Update the legacy parser’s license-item aggregation
around result_item.is_license_text so it preserves whether any matched lic_item
is license text instead of overwriting the flag per item. Use this aggregate
value both for result_item.is_license_text and the
filter_fsf_copyright_from_gpl_license_text call, and add a regression test
covering multiple license items in differing orders.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cd6b2a76-a679-4b32-b2c4-bea5282f5dfa
📒 Files selected for processing (2)
src/fosslight_source/_parsing_scancode_file_item.pytests/test_tox.py
OR-combine matched_rule.is_license_text so FSF copyright filtering no longer depends on the last license item order.
Uh oh!
There was an error while loading. Please reload this page.