Migrate Gradle samples to use new DV API #1119
Conversation
| server = 'https://develocity-samples.gradle.com' // adjust to your Develocity server | ||
| allowUntrustedServer = false // ensure a trusted certificate is configured | ||
|
|
||
| buildScan { | ||
| capture { taskInputFiles = true } |
There was a problem hiding this comment.
I understand that those flags are not necessary because set to the default value.
Do we want to apply the same rule to allowUntrustedServer ?
There was a problem hiding this comment.
This is a good point. I think the reason behind having allowUntrustedServer here is that it may have been set to true during a trial period and therefore we want to ensure that it is set to false for production. This is a good discussion though, should we be more explicit about the other fields?
There was a problem hiding this comment.
I think sharing a streamlined version with only mandatory fields make sense
There was a problem hiding this comment.
I think sharing a streamlined version with only mandatory fields make sense
I would agree with making this particular sample streamlined (i.e. don't explicitly declaring defaults). We have other material that can help a user determine whether they need allowUntrustedServer or not.
There was a problem hiding this comment.
So following this logic, we should also remove the buildCache.local.isEnabled=true logic. Do we agree on that?
There was a problem hiding this comment.
I would agree with that. I like the idea of this being the minimum viable recommended configuration. The less configuration there is the less chance for mistake.
There was a problem hiding this comment.
So following this logic, we should also remove the buildCache.local.isEnabled=true logic. Do we agree on that?
I could go either way. On one hand, removing it does match the criteria we apply to the develocity configuration. On the other hand, I find it somewhat common that users don't understand that org.gradle.caching=true enables the local build cache, so I don't mind leaving it here to be explicit. But then we're not being consistent 🙂
There was a problem hiding this comment.
Ok here's my proposal: let's discuss the buildCache.local.isEnabled=true and allowUntrustedServer = false changes separately and merge this PR as is.
| server = 'https://develocity-samples.gradle.com' // adjust to your Develocity server | ||
| allowUntrustedServer = false // ensure a trusted certificate is configured | ||
|
|
||
| buildScan { | ||
| capture { taskInputFiles = true } |
There was a problem hiding this comment.
I think sharing a streamlined version with only mandatory fields make sense
I would agree with making this particular sample streamlined (i.e. don't explicitly declaring defaults). We have other material that can help a user determine whether they need allowUntrustedServer or not.
| id 'com.gradle.common-custom-user-data-gradle-plugin' version '2' | ||
| } | ||
|
|
||
| def isCI = System.getenv('CI') != null // adjust to your CI provider | ||
|
|
||
| gradleEnterprise { | ||
| develocity { | ||
| server = 'https://develocity-samples.gradle.com' // adjust to your Develocity server | ||
| allowUntrustedServer = false // ensure a trusted certificate is configured | ||
|
|
||
| buildScan { |
There was a problem hiding this comment.
I suspect there is no hope in convincing you all of replacing:
buildScan {
uploadInBackground = !isCI
}With:
buildScan.uploadInBackground = !isCI
🙃
The samples were previously intentionally verbose. Given the new defaults for always publishing scans and always capturing input files, I think that we can simplify the configuration and reduce verbosity. Open to having a discussion about this.