[GitHub] [pinot] richardstartin commented on pull request #8016: Implement Real Time Mutable FST

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

richardstartin commented on pull request #8016:
URL: https://github.com/apache/pinot/pull/8016#issuecomment-1014335321


   > > I have two general points about the layout of the code (I don't like focussing on this sort of thing, but feel it's necessary here):
   > > 
   > > 1. It needs to be formatted properly: https://github.com/apache/pinot/blob/master/config/codestyle-intellij.xml
   > > 2. There are too many blank lines. In my humble opinion, these do not aid the reader at all. On many occasions I look at 15 lines of code and notice they compress to 5. It really obscures the logic by occupying screen space.
   > 
   > Thanks for pointing that out. The problem is that my laptop is M1 Pro based, so a global build of Pinot fails on it. I am dependent on Linter checks to tell me if check style is failing.
   
   I would certainly prefer to automate style checking because I want code to be readable automatically without needing to review it for readability. If a simple method doesn't fit on a screen without scrolling, it's unreadable. I've proposed using checkstyle to ban blank lines within functions to @Jackie-Jiang to help specifically with this case, but he wasn't too keen on the idea and reasoned that the _occasional_ blank line can help readability. 
   
   Regarding M1s, I think @dongxiaoman uses M1 and managed to build Pinot, maybe he can help?


-- 
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: commits-unsubscribe@pinot.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org