Fix checkstyle plugin, fix errors reported by checkstyle#234
Fix checkstyle plugin, fix errors reported by checkstyle#234deining wants to merge 3 commits intogoogle:mainfrom
Conversation
cpovirk
left a comment
There was a problem hiding this comment.
Thanks... and sorry :( I appreciate the effort here, but:
-
For formatting, we generally rely on http://github.com/google/google-java-format. It looks like this Checkstyle configuration wants some changes that would run counter to that. Checkstyle does also request some changes that we'd want. But we would probably want to either reformat the whole project or else reformat sections only as we touch them. (And I'm not sure which we'd want.)
-
For Maven Wrapper: That would be an improvement. However, for technical reasons, our internal build would require probably most of a day to set up with Maven Wrapper. That's doable, but it's hard to justify doing over lots of other things. Now, most of the cost for Maven Wrapper would be one-time setup. So, if someone does that setup someday, then we could pretty easily use it for Compile-Testing.
I have no reservations about the https changes and the changes in JavaFileObjects and MoreTrees. So if you want to have a PR that covers just those, we should be able to take that quickly.
From a previous discussion, I was invited to fix checkstyle currently not working properly. This PR is my take on that. What it does:
TODO: run checkstyle in CI, this is left up for you.