[GitHub] [commons-codec] aherbert commented on issue #27: Murmur3fix

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] [commons-codec] aherbert commented on issue #27: Murmur3fix

GitBox
aherbert commented on issue #27: Murmur3fix
URL: https://github.com/apache/commons-codec/pull/27#issuecomment-551274643
 
 
   As the author of the Eclipse formatting configuration in question I would state that I provided it for demo purposes to make formatting a bit easier on a PR. It is by no means a fire and forget configuration. I never use it on an entire file for commons code as it makes no allowances for changing preferences, for example on indentation for method parameters on new lines. Eclipse also mangles HTML in javadoc comments, particularly list elements <li>. So it would not help with a big formatting inconsistency in the javadoc that is often seen across most shared codebases.
   
   I use automated eclipse formatting on other projects with copious amounts of `@formatter:off` tags to prevent formatting code that has been laid out as easy to read. This litters a project with specific tags and I would be wary of doing this for a repository with a lot of contributors with different work set-ups for IDEs, etc.
   
   I like automated tools to do mindless jobs. But code is formatted with the express purpose of being easy to read (compilers don't care) so at least leave the option to allow human formatting to overrule the formatter.
   
   I prefer the option of using a very strict set of checkstyle rules. You can then maintain consistency without having to use an auto-formatter which may rearrange your carefully laid out array declarations, etc. A case in point is that checkstyle for this project should be set up to refuse tabs in source code which arrived in MurmurHash3 and its tests. This would be missed in a PR review as Github converts tabs to whitespace when showing the diff view.
   
   I think a start point would be updating checkstyle to use all the latest rules. Just updating the checkstyle version is not enough, you need to pull in the new rules too.
   

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


With regards,
Apache Git Services