ProblemReporter methods take all mandatory info as arguments #31854
Conversation
This file contains hidden or 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
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1bcb568 to
ef71a9d
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
e2cc233 to
11eac63
Compare
hegyibalint
approved these changes
Jan 2, 2025
| public void execute() { | ||
| Exception wrappedException = new Exception("Wrapped cause"); | ||
| getProblems().getReporter().throwing(${targetVersion >= GradleVersion.version('8.13') ? 'new RuntimeException("Exception message", wrappedException), ' : ''}problem -> problem | ||
| getProblems().getReporter().throwing(${targetVersion >= GradleVersion.version('8.13') ? 'new RuntimeException("Exception message", wrappedException), ' + ProblemsApiGroovyScriptUtils.createIdExpression() + ', ' : ''}problem -> problem |
There was a problem hiding this comment.
💭 This is not exactly what I meant – if your change is based on my feedback. My idea is to create completely separate, even if repetitive, methods for reporting a problem. In this case, that would be from L140:144.
What you did is also fine; I'm just expressing that this wasn't my exact idea.
| String contextualMessage = String.format("The 'tools.jar' cannot be found in the JDK '%s'.", jvm.getJavaHome()); | ||
| throw problemReporter.throwing(new IllegalStateException(contextualMessage), problemSpec -> problemSpec | ||
| .id(ProblemId.create("groovy-daemon-compiler", "Missing tools.jar", GradleCoreProblemGroup.compilation().groovy())) | ||
| ProblemId problemId = ProblemId.create("groovy-daemon-compiler", "Missing tools.jar", GradleCoreProblemGroup.compilation().groovy()); |
There was a problem hiding this comment.
As I said, I have no strong preferences over here. I still think this is a sort of infrastructure problem, and this doesn't communicate that well. OTOH the importance of this is really low.
It's more about thinking and establishing best practices, which we could document for the larger community.
| // This needs to be Java 6 compatible, as we are in a worker | ||
| getProblems().getReporter().report(problem -> problem | ||
| .id(org.gradle.api.problems.ProblemId.create("type", "label", org.gradle.api.problems.ProblemGroup.create("generic", "Generic"))) | ||
| getProblems().getReporter().report(org.gradle.api.problems.ProblemId.create("type", "label", org.gradle.api.problems.ProblemGroup.create("generic", "Generic")), problem -> problem |
There was a problem hiding this comment.
You gave a 👍 but I don't see any change related to this. Can you confirm?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
c793ec2 to
22718a4
Compare
This comment has been minimized.
This comment has been minimized.
22718a4 to
265a6dd
Compare
This comment has been minimized.
This comment has been minimized.
265a6dd to
979cf3e
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
The following builds have failed: |
979cf3e to
7d156fb
Compare
|
@bot-gradle merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
In other words, the definition of
ProblemIdis moved fromProblemSpecto a method argument ofProblemReportermembers.Context
Reviewing cheatsheet
Before merging the PR, comments starting with