dsmiley commented on code in PR #2571: URL: https://github.com/apache/solr/pull/2571#discussion_r1684972908
########## solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java: ########## @@ -124,7 +119,10 @@ private ClusterState fetchClusterState( throws SolrServerException, IOException, NotACollectionException { ModifiableSolrParams params = new ModifiableSolrParams(); if (collection != null) { + log.debug("Making a call to Solr to fetch cluster state for collection: ", collection); Review Comment: Why do you say that Eric? If it was needed, the precommit validation would tell her. Part of the value of the PR validation is that we don't need to waste any time telling contributors something that the automation will inform them to do. (similar for code formatting with tidy) In this case, the if-check is *not* needed because this log call is simple -- no method call inside the arguments. If the parameter there was foobar.callingAMethod().toString() or some-such, *then* the precommit check would say to add a condition. Even though the verbosity of adding two lines for the isDebugEnabled check is minor, it annoys me -- I don't want to see it added if the validation doesn't ask us to. -- 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