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

Reply via email to