-
Notifications
You must be signed in to change notification settings - Fork 308
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
Yet more CopyrightStatementsProcessor
improvements
#6022
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportBase: 65.48% // Head: 58.04% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #6022 +/- ##
============================================
- Coverage 65.48% 58.04% -7.45%
- Complexity 2003 2013 +10
============================================
Files 323 324 +1
Lines 16576 18699 +2123
Branches 1976 3847 +1871
============================================
- Hits 10855 10854 -1
- Misses 4661 6784 +2123
- Partials 1060 1061 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
f9ff3df
to
71a2bcd
Compare
71a2bcd
to
47e61e9
Compare
Should this be moved back to "draft" state, as it introduces copyright statements without holder into e.g. the notices? |
This comment was marked as outdated.
This comment was marked as outdated.
…processed Previously, these were neither stored as processed nor as unprocessed. For transparency, store these now as unprocessed, and add a test to ensure that each statement is either processed or unprocessed. Maintaining "stubs" without an owner, like "Copyright (c) 2006", keeps ORT on the safe side with respect to some license obligation like the BSD-3-Clause's "Redistributions in binary form must reproduce the above copyright notice", which strictly speaking also applies to such "stubs". Signed-off-by: Sebastian Schuberth <[email protected]>
Any code that compares `Parts` as thus makes use of the data class's auto-generated `equals()` function should not take the list of `originalStatements` into account. Avoid that pitfall by rewriting the code to not require `originalStatements` to be stored within `Parts`. This implicitly also aligns the semantics of `compareTo() == 0` with `equals() == true`, which is important to avoid unexpected behavior when storing `Parts` in a `SortedSet`. Signed-off-by: Sebastian Schuberth <[email protected]>
47e61e9
to
94e6c2b
Compare
@fviernau, I finally found the time to continue working on this. In the meantime, I had verified with @LeChasseur that, to be on the safe side, even Copyright "stubs" without a holder, like "Copyright (c) 2006", should be maintained to fully satisfy e.g. And the second commit also proves to be the right thing in the context of the recent |
I fear I won't find the time to do a review prior to my X-mas holidays. Is this urgent? |
It's not super urgent. But it's also not a lot of code. If you agree with the basic idea, and trust the test coverage, then also anyone else could approve, if you're fine with that. I just wanted to give you a heads up. |
Code looks good to me, but I wonder if the outcome of the change should be discussed in a community meeting first. I'd also like to discuss this internally at Bosch before the merge but that won't be possible before next year. |
Needs to be discussed in EPAM as well after the holidays. |
Is this something that could be fixed upstream in ScanCode instead? |
I don't think so. ScanCode already does report Copyright statements without holders (if that's how they're declared in the source), but our post-processing so far removed these. So it's something we need to tackle in ORT. |
Please have a look at the individual commit messages for the details.