-
Notifications
You must be signed in to change notification settings - Fork 5k
Allow public API to declare Problem IDs in a unified way #31778
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
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
one question
* @return the instance | ||
* @since 8.13 | ||
*/ | ||
public static IdFactory instance() { |
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 be a service?
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.
That's the interesting bit in the review. My stance is that problem Ids and groups are lightweight descriptors that Gradle and third party plugins both should be able to define as static data. ie:
class MyProblemIds {
public static final ProblemGroup group = ...
public static final ProblemId id1 = ...
public static final ProblemId id2 = ...
}
I played around with making it a service but it made the the whole Problems API usage a fair bit clunky to use.
...s/ide/problems-api/src/main/java/org/gradle/api/problems/internal/DefaultProblemBuilder.java
Show resolved
Hide resolved
...onTest/groovy/org/gradle/integtests/tooling/r812/ProblemProgressEventCrossVersionTest.groovy
Show resolved
Hide resolved
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.
641b659
to
149def4
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
149def4
to
d0dfd7e
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.
d0dfd7e
to
28ced5e
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
28ced5e
to
ef3c29e
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: |
The following builds have failed: |
ef3c29e
to
60279d3
Compare
@bot-gradle merge |
This is a PR to review changes at #31674 in reviewable steps.
Reviewing cheatsheet
Before merging the PR, comments starting with