-
Notifications
You must be signed in to change notification settings - Fork 33
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
Migrated a gradle groovy sample to develocity plugin api #1331
Conversation
@@ -12,7 +12,7 @@ import java.util.stream.Collectors | |||
* adding a flag as custom value. | |||
*/ | |||
|
|||
def buildScanApi = project.extensions.findByName('buildScan') | |||
def buildScanApi = project.extensions.findByName('develocity').getBuildScan(); |
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.
This is different than how we do it in all other samples.
Lines 9 to 12 in b84ea9b
def buildScanApi = project.extensions.findByName('buildScan') | |
if (!buildScanApi) { | |
return | |
} |
I believe this will throw a NPE if the develocity
extension doesn't exist, though it's unlikely you would be applying this with the Develocity Gradle plugin applied.
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, I know it's different, but I'm not sure how to do it differently in groovy, as doing what was there before get's you an error: Could not find matching constructor for: Capture(com.gradle.enterprise.gradleplugin.internal.extension.b_Decorated, org.gradle.internal.logging.slf4j.OutputEventListenerBackedLogger)
- scan. I'll handle the develocity being null though, missed that.
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.
I see you pushed some changes. Does it fix the error you're describing?
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.
Nope, it just fixes the case where develocity
extension is not available
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.
Okay so the script is still broken?
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.
No, the way I opened the PR is not broken - I wouldn't open a PR otherwise (it also shouldn't pass the tests if it were broken) :)
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.
Okay, I misunderstood what you said earlier.
Migrated a missed gradle groovy sample to develocity plugin api