+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é <j...@nanthrax.net> 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 > > <kirpic...@google.com.invalid> 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 > <dhalp...@google.com.invalid> > >> 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 <je...@smokinghand.com > > > >>> wrote: > >>> > >>>> Please. That's the one that always trips me up. > >>>> > >>>> On Tue, Aug 23, 2016, 4:10 PM Ben Chambers <bchamb...@apache.org> > >> 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é > jbono...@apache.org > http://blog.nanthrax.net > Talend - http://www.talend.com >
