dweiss commented on a change in pull request #277: URL: https://github.com/apache/solr/pull/277#discussion_r698541218
########## File path: build.gradle ########## @@ -20,7 +20,7 @@ import java.time.format.DateTimeFormatter plugins { id "base" - id "com.palantir.consistent-versions" version "1.14.0" Review comment: There is a patch upgrading this plugin (and all of gradle infrastructure) here - https://github.com/apache/solr/pull/275. It'll conflict with your PR, once applied. I'll cherry pick relevant areas if you don't want to spend any more time on it. ########## File path: gradle/generate-defaults.gradle ########## @@ -42,6 +42,7 @@ configure(rootProject) { "systemProp.file.encoding=UTF-8", "org.gradle.jvmargs=-Xmx3g", // TODO figure out why "gradlew check" runs out of memory if 2g "org.gradle.parallel=true", + "org.gradle.caching=true", Review comment: This enables local caches, right? I'm not sure folks will be enthusiastic about their home folders increasing in size so dramatically - solr build outputs are pretty heavy. ########## File path: gradle/validation/check-broken-links.gradle ########## @@ -40,11 +41,13 @@ class CheckBrokenLinksTask extends DefaultTask { @InputFile File script = project.rootProject.file("dev-tools/scripts/checkJavadocLinks.py") + @OutputFile Review comment: Yup, this is good, thanks. ########## File path: gradle/wrapper/gradle-wrapper.properties ########## @@ -1,5 +1,5 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-6.8.3-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-6.9.1-bin.zip Review comment: this alone won't work - requires a new checksum too (dont in that other PR). ########## File path: solr/solr-ref-guide/build.gradle ########## @@ -215,20 +216,21 @@ check.dependsOn checkLocalJavadocLinksSite, checkSite // Hook site building to assemble. assemble.dependsOn buildSite +@CacheableTask abstract class PrepareSources extends DefaultTask { // Original Source files we'll be syncing <b>FROM</b> @InputDirectory - public DirectoryProperty srcDir = project.objects.directoryProperty() + abstract DirectoryProperty getSrcDir() Review comment: Are these abstract properties automatically injected if you declare them like this? Where is the gradle docs that explains this (and which properties can be injected like so)? Thanks. ########## File path: solr/solr-ref-guide/build.gradle ########## @@ -184,6 +184,7 @@ ext { group "Documentation" description "Builds the ${details.desc}" + outputs.cacheIf { true } Review comment: I'm not sure what it does here? ########## File path: build.gradle ########## @@ -20,7 +20,7 @@ import java.time.format.DateTimeFormatter plugins { id "base" - id "com.palantir.consistent-versions" version "1.14.0" Review comment: I'm planning to merge tomorrow, sorry for not mentioning this earlier. ########## File path: solr/solr-ref-guide/build.gradle ########## @@ -215,20 +216,21 @@ check.dependsOn checkLocalJavadocLinksSite, checkSite // Hook site building to assemble. assemble.dependsOn buildSite +@CacheableTask abstract class PrepareSources extends DefaultTask { // Original Source files we'll be syncing <b>FROM</b> @InputDirectory - public DirectoryProperty srcDir = project.objects.directoryProperty() + abstract DirectoryProperty getSrcDir() Review comment: Ah, I see it now. Yeah - it'd be good to require the Inject annotation on those abstract properties too. This would be more intuitive (to me at least). ########## File path: gradle/generate-defaults.gradle ########## @@ -42,6 +42,7 @@ configure(rootProject) { "systemProp.file.encoding=UTF-8", "org.gradle.jvmargs=-Xmx3g", // TODO figure out why "gradlew check" runs out of memory if 2g "org.gradle.parallel=true", + "org.gradle.caching=true", Review comment: I think this should be left commented out in the generated user defaults - let it be an opt-in. I plan to commit gradle 7.2 path tomorrow and reiterate on your patch, cherry picking the obvious parts of it. Stay tuned. :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org