Preparations for removing ReporterInput.resolutionProvider
#8048
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #8048 +/- ##
=========================================
Coverage 67.07% 67.08%
Complexity 2054 2054
=========================================
Files 357 357
Lines 17120 17120
Branches 2457 2457
=========================================
+ Hits 11484 11485 +1
Misses 4613 4613
+ Partials 1023 1022 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
mnonnenmacher
requested changes
Dec 19, 2023
helper-cli/src/main/kotlin/commands/repoconfig/GenerateRuleViolationResolutionsCommand.kt
Outdated
Show resolved
Hide resolved
plugins/reporters/evaluated-model/src/funTest/assets/reporter-test-input.yml
Outdated
Show resolved
Hide resolved
Use the analog `OrtResult.getRuleViolations()` instead. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
The function only returns resolutions from the repository configurations, but not the other ones from the resolved configuration. This prepares for introducing a new `getResolutions()` implementation which returns all resolutions contained in the ORT result. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
Prepare for making use of it in a following change. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
Make the functions adhere to the resolutions from the resolved configuration instead of those from the repository configuration. This enables upcoming simplifactions in the callers of these function. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
Retrieve all resolutions from the provider earlier, and integrate them into the `OrtResult` before passing the `OrtResult` to the reporters. This obviously makes it unnecessary to pass the `ResolutionProvider` to the reporters and prepares for removing the corresponding property from `ReporterInput`, so that it does not get passed around anymore. Until then, just pass a `ResolutionProvider` constructed from `OrtResult`'s resolutions, to avoid querying the resolutions again. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
The reporter command has been changed to ensure that all resolutions are included in the `OrtResult` before passing it around to the reporters. Reflect that also in this test asset. This prepares for the removal of `ResolutionProvider` from the `EvaluatedModelMapper` / `StatisticsCalculator`. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
sschuberth
reviewed
Dec 20, 2023
| Issue(message = "Issue message to resolve", source = ""), | ||
| Issue(message = "Non-resolved issue", source = "") | ||
| ) | ||
| Identifier("Maven:org.oss-review-toolkit:example:1.0") to listOf( |
There was a problem hiding this comment.
Just for my understanding: Is this change necessary because the original resolution from the repo config was not actually "baked" into the resolutions from the resolved configuration in the hard-coded test asset, although it usually would be if the repo config is read by ORT?
There was a problem hiding this comment.
I.e. this is a bit similar to the adjustments of the test asset in your last commit 398a621, correct?
mnonnenmacher
approved these changes
Dec 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See individual commits.
Part of #7921.