+1 to import order I don't care about actually enforcing formatting, but would add it to IDE tips and just make it an "OK topic for code review". Enforcing it would result in obscuring a lot of history for who to talk to about pieces of code.
And by the way there is a recent build of the IntelliJ plugin for https://github.com/google/google-java-format, available through the usual plugin search functionality. I use it and it is very nice. On Tue, Aug 23, 2016 at 11:26 PM, Aljoscha Krettek <[email protected]> wrote: > +1 on the import order > > +1 on also starting a discussion about enforced formatting > > On Wed, 24 Aug 2016 at 06:43 Jean-Baptiste Onofré <[email protected]> wrote: > > > Agreed. > > > > It makes sense for the import order. > > > > Regards > > JB > > > > On 08/24/2016 02:32 AM, Ben Chambers wrote: > > > I think introducing formatting should be a separate discussion. > > > > > > Regarding the import order: this PR demonstrates the change > > > https://github.com/apache/incubator-beam/pull/869 > > > > > > I would need to update the second part (applying optimize imports) > prior > > to > > > actually merging. > > > > > > On Tue, Aug 23, 2016 at 5:08 PM Eugene Kirpichov > > > <[email protected]> wrote: > > > > > >> Two cents: While we're at it, we could consider enforcing formatting > as > > >> well (https://github.com/google/google-java-format). That's a bigger > > >> change > > >> though, and I don't think it has checkstyle integration or anything > like > > >> that. > > >> > > >> On Tue, Aug 23, 2016 at 4:54 PM Dan Halperin > > <[email protected]> > > >> wrote: > > >> > > >>> yeah I think that we would be SO MUCH better off if we worked with an > > >>> out-of-the-box IDE. We don't even distribute an IntelliJ/Eclipse > config > > >>> file right now, and I'd like to not have to. > > >>> > > >>> But, ugh, it will mess up ongoing PRs. I guess committers could fix > > them > > >> in > > >>> merge, or we could just make proposers rebase. (Since committers are > > most > > >>> proposers, probably little harm in the latter). > > >>> > > >>> On Tue, Aug 23, 2016 at 4:11 PM, Jesse Anderson < > [email protected] > > > > > >>> wrote: > > >>> > > >>>> Please. That's the one that always trips me up. > > >>>> > > >>>> On Tue, Aug 23, 2016, 4:10 PM Ben Chambers <[email protected]> > > >> wrote: > > >>>> > > >>>>> When Beam was contributed it inherited an import order [1] that was > > >>>> pretty > > >>>>> arbitrary. We've added org.apache.beam [2], but continue to use > this > > >>>>> ordering. > > >>>>> > > >>>>> Both Eclipse and IntelliJ default to grouping imports into > alphabetic > > >>>>> order. I think it would simplify development if we switched our > > >>>> checkstyle > > >>>>> ordering to agree with these IDEs. This also removes special > > >> treatment > > >>>> for > > >>>>> specific packages. > > >>>>> > > >>>>> If people agree, I'll send out a PR that changes the checkstyle > > >>>>> configuration and runs IntelliJ's sort-imports on the existing > files. > > >>>>> > > >>>>> -- Ben > > >>>>> > > >>>>> [1] > > >>>>> org.apache.beam,com.google,android,com,io,Jama,junit,net, > > >>>> org,sun,java,javax > > >>>>> [2] com.google,android,com,io,Jama,junit,net,org,sun,java,javax > > >>>>> > > >>>> > > >>> > > >> > > > > > > > -- > > Jean-Baptiste Onofré > > [email protected] > > http://blog.nanthrax.net > > Talend - http://www.talend.com > > >
