junit4 icon indicating copy to clipboard operation
junit4 copied to clipboard

Make some effort to comply with style guide

Open tomwhoiscontrary opened this issue 9 years ago • 0 comments

This relates to #1350, "Code base doesn't comply with JUnit's own coding style". It adds the Google Formatter to reformat the code, and Checkstyle to enforce compliance. It doesn't actually include a mass reformatting of code.

I'm not sure this is something you'd want to merge, but it might help understand the situation better.

Some observations:

  • JUnit differs from Google in that it uses four spaces from indentation. The Google Formatter can't do that. However, we can roughly restore four-space indentation with a fairly simple sed command (as given in the POM!). Also, we can suppress indentation-related violations in Checkstyle, so we won't fail the build because of our formatting.
  • By default, Checkstyle will only fail the build for 'errors', of which the codebase has none. However, the codebase has 1289 'warnings' (ignoring indentation), which still seem serious enough to want to fix.
  • Google Formatter actually doesn't fix all the problems Checkstyle finds. For example, it doesn't reorder imports. I get the impression that there is reformatting that Google Formatter can do on the command line that it can't do through its API, which is what the Maven plugin uses. After running the formatter, the codebase is down to 1113 warnings. However, after restoring indentation, it's back up to 1235 - even with indentation warnings ignored!

So, whilst you might well want Checkstyle as part of the build, i'm skeptical that the Google Formatter is actually useful.

tomwhoiscontrary avatar Oct 30 '16 23:10 tomwhoiscontrary