dsmiley commented on code in PR #2706:
URL: https://github.com/apache/solr/pull/2706#discussion_r1774499355


##########
build.gradle:
##########
@@ -139,7 +143,7 @@ apply from: file('gradle/maven/defaults-maven.gradle')
 
 // IDE support, settings and specials.
 apply from: file('gradle/ide/intellij-idea.gradle')
-apply from: file('gradle/ide/eclipse.gradle')
+// apply from: file('gradle/ide/eclipse.gradle')

Review Comment:
   do we remove it then?



##########
gradle/node.gradle:
##########
@@ -16,7 +16,7 @@
  */
 
 configure([project(":solr:packaging"), project(":solr:solr-ref-guide"), 
project(":solr:webapp")]) {
-    apply plugin: "com.github.node-gradle.node"
+    apply plugin: libs.plugins.nodegradle.node.get().pluginId

Review Comment:
   TBH I liked it as it was -- a well known string instead of some dotted 
namespace from the toml that we invent for each dependency.



##########
gradle/libs.versions.toml:
##########
@@ -0,0 +1,432 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+[versions]
+adobe-testing-s3mock = "2.17.0"
+amazon-awssdk = "2.26.19"
+# @keep Antora version used in ref-guide
+antora = "3.1.4"
+# @keep Most recent commit as of 2022-06-24, this repo does not have tags
+antora-default-ui = "51ad811622394027afb4e182c2fdabc235ae04dd"
+# @keep Antora Lunr extensions version used in ref-guide
+antora-lunr-extension = "1.0.0-alpha.8"
+apache-calcite = "1.37.0"
+apache-calcite-avatica = "1.25.0"
+apache-commons-collections4 = "4.4"
+apache-commons-compress = "1.26.1"
+apache-commons-configuration2 = "2.11.0"
+apache-commons-exec = "1.4.0"
+apache-commons-lang3 = "3.15.0"
+apache-commons-math3 = "3.6.1"
+# @keep for version alignment
+apache-commons-text = "1.12.0"
+apache-curator = "5.7.0"
+apache-hadoop = "3.3.6"
+apache-hadoop-thirdparty = "1.2.0"
+apache-httpcomponents-httpclient = "4.5.14"
+apache-httpcomponents-httpcore = "4.4.16"
+apache-httpcomponents-httpmime = "4.5.14"
+apache-kerby = "1.0.1"
+apache-log4j = "2.21.0"
+apache-lucene = "9.11.1"
+apache-opennlp = "1.9.4"
+apache-poi = "5.2.2"
+apache-rat = "0.15"
+apache-tika = "1.28.5"
+apache-tomcat = "6.0.53"
+apache-zookeeper = "3.9.2"
+aqute-bnd = "6.4.1"
+# @keep Asciidoctor mathjax version used in ref-guide
+asciidoctor-mathjax = "0.0.9"
+# @keep Asciidoctor tabs version used in ref-guide
+asciidoctor-tabs = "1.0.0-beta.6"
+bc-jose4j = "0.9.6"
+benmanes-caffeine = "3.1.8"
+benmanes-versions = "0.51.0"
+bouncycastle = "1.78.1"
+# @keep Browserify version used in ref-guide
+browserify = "17.0.0"
+carrot2-core = "4.5.1"
+carrotsearch-dependencychecks = "0.0.9"
+carrotsearch-hppc = "0.10.0"
+carrotsearch-randomizedtesting = "2.8.1"
+# @keep for version alignment
+checkerframework = "3.44.0"
+codehaus-woodstox = "4.2.2"
+commons-cli = "1.9.0"
+commons-codec = "1.17.1"
+commons-collections = "3.2.2"
+commons-io = "2.15.1"
+cutterslade-analyze = "1.10.0"
+cybozulabs-langdetect = "1.1-20120112"
+diffplug-spotless = "6.5.2"
+dropwizard-metrics = "4.2.26"
+eclipse-ecj = "3.33.0"
+eclipse-jetty = "10.0.22"
+eclipse-jettytoolchain = "4.0.6"
+# @keep jgit version used by git-statuts.gradle
+eclipse-jgit = "6.7.0.202309050840-r"
+fasterxml = "2.17.2"
+fasterxml-woodstox = "6.7.0"
+# @keep Flexmark used in classpath
+flexmark = "0.64.8"
+google-api-gax = "2.33.0"
+# @keep for version alignment
+google-api-grpc-proto = "2.42.0"
+google-auth = "1.19.0"
+# @keep for version alignment
+google-autovalue = "1.10.4"
+google-cloud-bom = "0.224.0"
+google-cloud-core = "2.23.0"
+google-cloud-nio = "0.127.3"
+google-cloud-storage = "2.27.0"
+google-errorprone = "2.28.0"
+# @keep for version alignment
+google-failureaccess = "1.0.2"
+# @keep for version alignment
+google-gson = "2.11.0"
+google-guava = "33.1.0-jre"
+# @keep This is GJF version for spotless/ tidy.
+google-javaformat = "1.18.1"
+# @keep for version alignment
+google-protobuf = "3.25.3"
+google-re2j = "1.7"
+# @keep Gradle version to run the build
+gradle = "8.4"
+grpc = "1.65.1"
+# @keep Gulp version used in ref-guide
+gulp-cli = "2.3.0"
+hamcrest = "2.2"
+hk2 = "3.0.5"
+hsqldb = "2.7.2"
+ibm-icu = "74.2"
+immutables-valueannotations = "2.10.1"
+j256-simplemagic = "1.17"
+jakarta-annotation = "2.1.1"
+jakarta-inject = "2.0.1"
+jakarta-ws = "3.1.0"
+javacc = "7.0.12"
+# @keep for version alignment
+jaxb = "2.3.8"
+jayway-jsonpath = "2.9.0"
+jctools = "4.0.5"
+jersey = "3.1.5"
+# TODO Sync with jersey versions
+jersey-containers = "2.39.1"
+# @keep for version alignment
+joda-time = "2.8.1"
+junit = "4.13.2"
+# @keep Link checker version used in ref-guide
+link-checker = "1.4.2"
+littlerobots-versioncatalogupdate = "0.8.4"
+lmax-disruptor = "3.4.4"
+ltgt-errorprone = "3.1.0"
+# @keep This is the minimum required Java version.
+minJava = "11"
+mockito = "5.12.0"
+morethan-jmhreport = "0.9.0"
+navsecurity = "0.5.10"
+netty = "4.1.112.Final"
+# @keep for version alignment
+netty-tcnative = "2.0.65.Final"
+nimbusds-josejwt = "9.30.2"
+nodegradle-node = "7.0.1"
+openapi = "7.6.0"
+openjdk-jmh = "1.37"
+opentelemetry = "1.40.0"
+osgi-annotation = "8.1.0"
+# @keep for version alignment
+ow2-asm = "9.3"
+owasp-dependencycheck = "9.0.8"
+# @keep for version alignment
+perfmark = "0.27.0"
+prometheus-metrics = "1.1.0"
+prometheus-simpleclient = "0.16.0"
+quicktheories = "0.26"
+semver4j = "5.3.0"
+slf4j = "2.0.13"
+spatial4j = "0.8"
+spotbugs = "4.8.0"
+squareup-okhttp3-mockwebserver = "4.11.0"
+squareup-okhttp3-okhttp = "4.12.0"
+stephenc-jcip = "1.0-1"
+swagger3 = "2.2.22"
+tdunning-tdigest = "3.3"
+thetaphi-forbiddenapis = "3.7"
+thisptr-jacksonjq = "0.0.13"
+threeten-bp = "1.6.8"
+undercouch-download = "5.5.0"
+xerces = "2.12.2"
+xerial-snappy = "1.1.10.5"
+
+[plugins]
+benmanes-versions = { id = "com.github.ben-manes.versions", version.ref = 
"benmanes-versions" }
+carrotsearch-dependencychecks = { id = 
"com.carrotsearch.gradle.dependencychecks", version.ref = 
"carrotsearch-dependencychecks" }
+cutterslade-analyze = { id = "ca.cutterslade.analyze", version.ref = 
"cutterslade-analyze" }
+diffplug-spotless = { id = "com.diffplug.spotless", version.ref = 
"diffplug-spotless" }
+littlerobots-versioncatalogupdate = { id = 
"nl.littlerobots.version-catalog-update", version.ref = 
"littlerobots-versioncatalogupdate" }
+ltgt-errorprone = { id = "net.ltgt.errorprone", version.ref = 
"ltgt-errorprone" }
+morethan-jmhreport = { id = "io.morethan.jmhreport", version.ref = 
"morethan-jmhreport" }
+nodegradle-node = { id = "com.github.node-gradle.node", version.ref = 
"nodegradle-node" }
+openapi-generator = { id = "org.openapi.generator", version.ref = "openapi" }
+owasp-dependencycheck = { id = "org.owasp.dependencycheck", version.ref = 
"owasp-dependencycheck" }
+swagger3-core = { id = "io.swagger.core.v3.swagger-gradle-plugin", version.ref 
= "swagger3" }
+thetaphi-forbiddenapis = { id = "de.thetaphi.forbiddenapis", version.ref = 
"thetaphi-forbiddenapis" }
+udnercouch-download = { id = "de.undercouch.download", version.ref = 
"undercouch-download" }
+
+[libraries]

Review Comment:
   ah well, we've got to follow what Gradle wants us to do now but IMO coming 
up with a unique library string for each library is annoying.  Maintaining this 
is annoying.



##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java:
##########
@@ -85,7 +85,7 @@ public class SolrZkClient implements Closeable {
   private int minStateByteLenForCompression;
 
   public MapWriter getMetrics() {
-    return metrics::writeMap;
+    return metrics;

Review Comment:
   Not needed in this PR, I assume



##########
help/dependencies.txt:
##########
@@ -29,60 +29,126 @@ testImplementation - makes a dependency only available for 
test  classes.
 Adding a library dependency
 ---------------------------
 
-Let's say we wish to add a dependency on library "foo.bar:baz" in
+Let's say we wish to add a new dependency on library "foo.bar:baz" in
 version 1.2 to :solr:core. Let's assume this library is only
-used internally by the project. The :solr:core project is configured
-by solr/core/build.gradle and we would add (or modify) the dependency
-block as follows:
+used internally by the project. For new dependencies, we would add
+the dependency and its version to gradle/libs.versions.toml first:
+
+[versions]
+...
+foo-bar-baz = "1.2"
+...
+
+[libraries]
+...
+foo-bar-baz = { module = "foo.bar:baz", version.ref = "foo-bar-baz" }
+
+Note that the used names separated by dashes are later referenced with dots
+instead of dashes, but more on that later.
+
+The chosen name for the module should more or less reflect the module's
+group name and module id in a way that it groups related dependencies under
+the same "prefix" (see below). There is no specific convention here and
+group prefixes for domain names like "com" and "io" are avoided, as they
+do not add any value and increase the size of the reference / alias name.

Review Comment:
   to use libs.versions.toml; are we actually forced to add such invented names 
here and use them as such as well?



-- 
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

Reply via email to