mynameborat commented on issue #1047: SAMZA-2210: Initial majority migration 
for injecting classloader when doing reflection 
URL: https://github.com/apache/samza/pull/1047#issuecomment-498425195
 
 
   > > Looks good.
   > > One general question around code style, I noticed some of the 
methods/callsite have one argument per line while others have them crunched 
together to fit the width limit. This is prevalent across the code base but I 
noticed you changed some of them too. So wondering what the guideline is?
   > 
   > My personal preference is that "long" argument lists have the format of 
one argument per line. "Long" can be pretty subjective, but I usually consider 
argument lists that take up 3 or more lines as "long". The reason why I prefer 
this style is because future changes to the argument lists are easier to diff 
(e.g. diff has a single-line addition instead of possibly having multiple lines 
updated due to overflow of line length). I will sometimes make those style 
changes when I need to change an argument list.
   > All that said, I'm not sure what the general Samza style guideline is 
around this.
   
   Sounds good! We have a coding guideline that touches some areas but not 
comprehensive. At least that is the my motivation behind these guidelines/style 
questions. I'd be happy to consolidate our guidelines into a comprehensive wiki 
and share it.
   Let me know what do you think?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to