-
Notifications
You must be signed in to change notification settings - Fork 314
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
Simplify redundant SPDX choices #9641
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9641 +/- ##
=========================================
Coverage 68.05% 68.05%
Complexity 1285 1285
=========================================
Files 249 249
Lines 8835 8835
Branches 921 921
=========================================
Hits 6013 6013
Misses 2433 2433
Partials 389 389
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
"remove redundant choices" { | ||
val expression = "Apache-2.0 AND (Apache-2.0 OR MIT) AND MIT".toSpdx() | ||
|
||
// Compare string representations to not rely on semantic equality. |
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.
Should this say "structural equality" instead? I'm actually not sure if I understand the comment.
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.
As this is about simplifying expressions, I want to be sure that e.g. MIT AND MIT
is simplified to exactly MIT
although it is "MIT AND MIT".toSpdx() == "MIT".toSpdx()
.
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.
Yes, but isn't "MIT AND MIT" semantically equal with "MIT", but not syntactically?
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.
Yes, exactly. The comment is meaning to say: "If we would not use string-comparison here, expressions would be compared for being semantically equal, which is not what we want."
With sorting applied, the string representation can be hard-coded. Signed-off-by: Sebastian Schuberth <[email protected]>
Be extra sure that the actual string representation becomes simpler. Signed-off-by: Sebastian Schuberth <[email protected]>
551e4df
to
23eba66
Compare
Disambiguate the AND-operator's test name accordingly. Signed-off-by: Sebastian Schuberth <[email protected]>
While this was partly explained in the commit message that introduced the code, make this more visible. Signed-off-by: Sebastian Schuberth <[email protected]>
Extract flattening to a function and reuse it to either flatten an only choice, or the original expression. Signed-off-by: Sebastian Schuberth <[email protected]>
If there is a "choice" between "a" or "a", that is not really a choice. Signed-off-by: Sebastian Schuberth <[email protected]>
23eba66
to
e33e141
Compare
PS: It occurred to me that we probably could convert several functions, like |
Please have a look at the individual commit messages for the details.