Re: Remove legacy import-order?
Done as of #829. Thanks Ben, for simplifying our imports and getting rid of our legacy dependence on Google's import structure. On Wed, Aug 24, 2016 at 10:04 AM, Mark Liu wrote: > +1 for reformatting import order. > > On Wed, Aug 24, 2016 at 9:17 AM, Lukasz Cwik > wrote: > > > +1 for import order > > > > On Wed, Aug 24, 2016 at 9:01 AM, Amit Sela 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 > > > 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é > > > > > 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 > > > > > > > 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 > > > > > > > > > > > > >> 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 > > > > > > > > > > > > > > > > > > > > >
Re: Remove legacy import-order?
+1 for reformatting import order. On Wed, Aug 24, 2016 at 9:17 AM, Lukasz Cwik wrote: > +1 for import order > > On Wed, Aug 24, 2016 at 9:01 AM, Amit Sela 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 > > 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é > > > 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 > > > > > > 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 > > > > > > > > > > >> 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 > > > > > > > > > > > > > > >
Re: Remove legacy import-order?
+1 for import order On Wed, Aug 24, 2016 at 9:01 AM, Amit Sela 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 > 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 > > 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é > > 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 > > > > > 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 > > > > > > > > >> 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 > > > > > > > > > >
Re: Remove legacy import-order?
+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 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 > 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é > 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 > > > > 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 > > > > > > >> 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 > > > >> 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 > > > > > >
Re: Remove legacy import-order?
+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 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é 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 > > > 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 > > > > >> 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 > > >> 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 > > >
Re: Remove legacy import-order?
+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é 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 > > 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 > > >> 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 > > >>> wrote: > >>> > Please. That's the one that always trips me up. > > On Tue, Aug 23, 2016, 4:10 PM Ben Chambers > >> 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 >
Re: Remove legacy import-order?
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 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 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 wrote: Please. That's the one that always trips me up. On Tue, Aug 23, 2016, 4:10 PM Ben Chambers 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
Re: Remove legacy import-order?
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 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 > 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 > > wrote: > > > > > Please. That's the one that always trips me up. > > > > > > On Tue, Aug 23, 2016, 4:10 PM Ben Chambers > 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 > > > > > > > > > >
Re: Remove legacy import-order?
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 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 > wrote: > > > Please. That's the one that always trips me up. > > > > On Tue, Aug 23, 2016, 4:10 PM Ben Chambers 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 > > > > > >
Re: Remove legacy import-order?
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 wrote: > Please. That's the one that always trips me up. > > On Tue, Aug 23, 2016, 4:10 PM Ben Chambers 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 > > >
Re: Remove legacy import-order?
Please. That's the one that always trips me up. On Tue, Aug 23, 2016, 4:10 PM Ben Chambers 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 >
