+1 to blanket reformat as well. On Tue, Feb 21, 2017 at 1:25 PM, Otto Fowler <ottobackwa...@gmail.com> wrote:
> +1. I agree with Michael’s points. > > > On February 21, 2017 at 16:23:21, Michael Miklavcic ( > michael.miklav...@gmail.com) wrote: > > +1 to a blanket reformat, failed build for improper formatting, and > automated formatting. I strongly prefer to remove "thinking" from my code > formatting and it has worked very well for me on large projects in the > past. There is capability now in IntelliJ to work with Checkstyle as well. > https://youtrack.jetbrains.com/issue/IDEA-61520#comment=27-1292600 > https://plugins.jetbrains.com/idea/plugin/1065-checkstyle-idea > > A quick search didn't yield any obviously robust tools for automating the > formatting other than an older non-maintained project named Jalopy. I think > the checkstyle integration with IntelliJ and Eclipse should suffice since > the Maven plugin would give devs the ability to run checks locally and in > Github via Travis. > > > On Tue, Feb 21, 2017 at 12:32 PM, Nick Allen <n...@nickallen.org> wrote: > > > I would be in favor of a blanket, reformat. Whether that is for the > entire > > code base or one project at a time. Might be able to conquer and divide > > some of the heavy-lifting of testing, if we do a project at a time. But > > whichever way you think is easier. I'd be glad to help. > > > > On Tue, Feb 21, 2017 at 1:57 PM, Justin Leet <justinjl...@gmail.com> > > wrote: > > > > > I already tried a blanket, manual reformat the other day, through > > > IntelliJ. I did every file matching *.java in the project and it was > > > pretty quick. I didn't validate everything looked perfect afterwards, > > but I > > > did click into a few files and things looked fine. I'm not quite sure > > what > > > the lifecycle of our autogenerated stuff is, so we'd want to regen > > > afterwards, but it's a pretty trivial thing to do. > > > > > > I'm sure there's more nuance (and definitely more testing) than that, > but > > > off the top of my head I'm not sure what it would be. Either way, I > don't > > > think there's a huge amount of effort to just do the reformat, but we'd > > > still want to spin everything up and test it and so on. It's probably > > more > > > work for everybody to rebase onto the (vastly) reformatted code than > > > anything else, which will vary pretty significantly. > > > > > > For (slight) context, the changes are enough to eliminate ~5k > checkstyle > > > warnings (and there might be more if we have to tweak anything in the > > code > > > formatting). > > > > > > On Tue, Feb 21, 2017 at 10:34 AM, Casey Stella <ceste...@gmail.com> > > wrote: > > > > > > > Any idea, with those modifications to checkstyle, how much effort it > > will > > > > take to reformat the code to conform? > > > > > > > > On Tue, Feb 21, 2017 at 8:23 AM, Justin Leet <justinjl...@gmail.com> > > > > wrote: > > > > > > > > > As part of: > > > > > https://issues.apache.org/jira/browse/METRON-726 > > > > > https://github.com/apache/incubator-metron/pull/459 > > > > > > > > > > I integrated checkstyle into the mvn:site command, and have > > checkstyle > > > > > reports being run as part of the mvn:site reporting. I expect to be > > > > > celebrating hitting 25k checkstyle warnings soon. > > > > > > > > > > I tested out creating a code formatting setup in IntelliJ, with a > > > couple > > > > > slight modifications of the default Sun conventions (extended the > > > > character > > > > > limit of a line past 80 and made it two space indents). Given that > > > > > checkstyle includes it as a default option, it's probably > reasonably > > > > close > > > > > to the Sun conventions. I'm thinking we probably also at least > create > > > an > > > > > Eclipse profile, to open up ease of development. > > > > > > > > > > There's probably also a discussion about how exactly we want to > > enforce > > > > it. > > > > > Is it just something we add to the PR checklist and have reviewers > > > give a > > > > > glance, do we setup a hook to autoformat code, etc? > > > > > > > > > > Justin > > > > > > > > > > > > > > >
