[GitHub] [hbase] ndimiduk commented on pull request #4312: HBASE-26899 Run spotless:apply

[GitBox <gi...@apache.org>]

ndimiduk commented on PR #4312:
URL: https://github.com/apache/hbase/pull/4312#issuecomment-1098042757

   I have not had a chance to review the formatter changes, but from browsing my email notifications,
   
   > So my proposal: move "create" to second line, or condense it so that it's not so many lines. One or the other.
   
   +1
   
   > In the same file, similar thing here but with method chaining. Personally I'd prefer that we move .group(eventLoopGroup) to next line. It just makes it a bit easier to scan all the chained methods.
   
   +1
   
   > In TestRecoverLeaseFSUtils.java, this math + comparison ended up splitting somewhat awkwardly.
   
   I agree on principal that we should avoid splitting the paren grouping if possible, but I'm also not a fan of operand-operator-operand on new lines. Maybe something like this is possible.
   
   ```
   +    assertTrue((EnvironmentEdgeManager.currentTime() - startTime)
   +        > (3 * HTU.getConfiguration().getInt("hbase.lease.recovery.dfs.timeout", 61000)));
   ```
   
   The above happens to be consistent with my preferred formatting of the ternary operator, placing the operator symbols as the start of their own lines.
   
   > I think this format is more readable (from BackupCommands.java)
   
   +1


-- 
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-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org