kfaraz commented on PR #15706:
URL: https://github.com/apache/druid/pull/15706#issuecomment-1899813336

   @fectrain , yes, I agree that the changes here are simple enough to not 
cause any issue. But the problem is that it would also reduce the overall 
coverage of these classes because of the lines that are now inside the 
`log.isDebugEnabled` if clause. That might cause further build failures down 
the line.
   
   Enabling debug logging in tests doesn't seem to be a great option either as 
it would create unnecessary logs when running tests.
   
   See if you can reduce the number of lines in some cases. For example, the 
following code:
   ```
   log.debug(                             
       "ZKNode created for server to [%s] %s [%s]",
       basePath,
       segmentHolder.getAction(),
       segmentHolder.getSegmentIdentifier()
   );
   ```
   can be rewritten as
   ```
   log.debug(                             
       "ZKNode created for server to [%s] %s [%s]",
       basePath, segmentHolder.getAction(), segmentHolder.getSegmentIdentifier()
   );
   ```
   without compromising on readability.
   
   I know it's not a great solution but should help with the coverage to some 
extent.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to