Use of google-java-format before check-ins?

[Jeffrey Walton]

Hi Everyone/Kevin,

I am working on a few patches to offer back to the project. I want to
clean up line widths and whitespace, especially in Javadoc comments.
I'm not sure why it piqued my interest this time around.

google-java-format [0] does a good job of consistent formatting using
Java style recommendations. However, I cannot set a line width. It
is fixed at 100 for the tool. Google AI tells me the fixed parameters
are a design choice to ensure consistent formatting by everyone.

Is anyone opposed to using 100 columns when tidying up a Java source file?

[0] https://github.com/google/google-java-format

Jeff

[Kevin W. Wall]

Jeff, 

I think in an age where people seldom print out source code for code reviews (or other purposes) anymore, it rarely matters. I think it certainly matters much less than it did when the ESAPI Project first started (in 2007, I think). When I first started on ESAPI in 2009, I used vim and gvim and set the text width 80, in part because I was used to reviewing my own code on dead trees, and printing in portrait rather than landscape was my preferred page orientation. But as ESAPI evolved and more contributors started using IDEs, that 80 character width went by the wayside. Fast forward to today, when I now normally use terminal windows of 132 characters and I have ':set tw=132' in my ~/.exrc file.

So, I learned that times change and everyone has their own preferences. Now, when I edit any existing file in any repo, I try to honor the predominant style of what that source file already uses. In other words, I try not to be overly anal about stylistic choices like I was more in my youth. (Of course, my original anal tendencies can somewhat be excused as my first programming language was FORTRAN IV entered on an IBM 029 Key Punch on an 80 column cards, so it mattered a lot more back then. 😜 And, yes, I am that old! 🦖)

So, if you are doing a PR for ESAPI, my advice and personal preference is to follow the style that dominates in the file you are editing. And if you create a new file, choose whatever style you like (but please use spaces rather than tabs for indentation as that makes it simpler for us to change in the future if the code gets reorganized). However, one thing we prefer not to see is to see hundreds of lines split and/or combined for only stylistic reasons (and I'd consider reformatting Javadoc along those lines since when it's translated to HTML, it really shouldn't matter) that are without semantic significance. It makes the code review process of the PR more difficult than it needs to be.

@Matt Seil, @Jeremiah J. Stacey - what do you think? Do you have anything to add?

-kevin

P.S.- In hindsight, it probably would have been a good idea to suggest, and maybe even enforce,  some style guides when ESAPI started. This is something we should probably consider for ESAPI 3, but I think it's rather late to worry about the existing code for now. If we carry any of the ESAPI 2.x code forward to ESAPI 3, that is when we should consider it.

--

Blog: https://off-the-wall-security.blogspot.com/    | GitHub: @kwwall | OWASP ESAPI Project co-lead | OWASP and ACM lifetime member

NSA: All your crypto bit are belong to us.

--
You received this message because you are subscribed to the Google Groups "ESAPI Project Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to esapi-project-u...@owasp.org.
To view this discussion visit https://groups.google.com/a/owasp.org/d/msgid/esapi-project-users/CAH8yC8nwoNc96ZtJ%3Dz_oJ37oP76kW5qMKcWP_kE3iSUK7UmYRw%40mail.gmail.com.

[xeno...@gmail.com]

I have tended to do exactly as you have pointed out here.  I try to obey the file, only really mucking with it if it was clearly mixed or someone committed the tab instead of space party foul.  I do most of my console work on near full screen consoles these days 😅 and where I work every single team follows their own styles so I learned to not be so anal myself.  

 

I very often rewrite (Java) code to be more transactional to ease my own debugging life so that’s a long-winded way of saying do as thou wilt, just avoid tabs and do NOT mix code changes with style changes.  Keep those totally separate PRs. 

-- Matt Seil 

Manager/Princ Application Security 

Member ACM/OWASP