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 > > > > > >
