+1 to longer line lengths, beer on fridays and mother's apple pie. On Wed, Feb 22, 2017 at 5:01 AM, Kyle Richardson <kylerichards...@gmail.com> wrote:
> +1 to longer line lengths and blanket reformatting. Personally, I see IDE > integration as a must for adoption of checkstyle. > > -Kyle > > On Wed, Feb 22, 2017 at 1:39 AM, Matt Foley <ma...@apache.org> wrote: > > > +1, so do I. Also like the idea of providing the necessary IntelliJ > > specification. > > > > On 2/21/17, 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 > > > > > > > > > > > > > > > > > > > > > > > > > > >
