+1 for reformatting import order. On Wed, Aug 24, 2016 at 9:17 AM, Lukasz Cwik <lc...@google.com.invalid> wrote:
> +1 for import order > > On Wed, Aug 24, 2016 at 9:01 AM, Amit Sela <amitsel...@gmail.com> wrote: > > > +1 on import order as well. > > Kenneth has a good point about history if we reformat. > > > > On Wed, Aug 24, 2016, 18:59 Kenneth Knowles <k...@google.com.invalid> > > wrote: > > > > > +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 < > aljos...@apache.org> > > > 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é <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 > > > > > > > > > > > > > > >
