refactor: Remove some unnecessary uses of sorted sets #8700
Conversation
| scopeMappings.keys.toSortedSet().forEach { scope -> | ||
| orderedMappings[scope] = scopeMappings.getValue(scope) | ||
| scopeMappings.entries.sortedBy { it.key }.forEach { (scope, rootDependencyIndices) -> | ||
| orderedMappings[scope] = rootDependencyIndices |
There was a problem hiding this comment.
Hmm, I believe we need a different rationale here, as the code does not really look simpler to me, but longer and more complex. What's the real goal here, I guess to remove the use of toSortedSet()? Then we should say so.
There was a problem hiding this comment.
My goal was indeed to simplify. I've just dropped this commit.
There was a problem hiding this comment.
While reviewing the code base as part of #8705 I've come across this place again, and I've changed my mind a bit:
While the new code still is not really simpler IMO, it's better as it avoids an unneeded getValue() on scopeMappings (and the mention of toSortedSet() which we prefer to get rid of).
So, mind re-introducing the commit with a slightly adjust commit message?
There was a problem hiding this comment.
Sure, here you go: #8706.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8700 +/- ##
=========================================
Coverage 67.85% 67.85%
Complexity 1165 1165
=========================================
Files 244 244
Lines 7736 7736
Branches 865 865
=========================================
Hits 5249 5249
Misses 2128 2128
Partials 359 359
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Frank Viernau <[email protected]>
fviernau
force-pushed
the
unneeded-sorted-things
branch
from
da8af4f
to
7cd4296
Compare
See individual commits.