You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@beam.apache.org by Ben Chambers <bc...@apache.org> on 2016/08/23 23:09:48 UTC
Remove legacy import-order?
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?
Posted by Dan Halperin <dh...@google.com.INVALID>.
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 <ma...@google.com.invalid>
wrote:
> +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 <am...@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 <kl...@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 <
> > aljoscha@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é <jb@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
> > > > > > > <ki...@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
> > > > > > <dh...@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 <
> > > > > jesse@smokinghand.com
> > > > > > >
> > > > > > >>> wrote:
> > > > > > >>>
> > > > > > >>>> Please. That's the one that always trips me up.
> > > > > > >>>>
> > > > > > >>>> On Tue, Aug 23, 2016, 4:10 PM Ben Chambers <
> > > bchambers@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é
> > > > > > jbonofre@apache.org
> > > > > > http://blog.nanthrax.net
> > > > > > Talend - http://www.talend.com
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: Remove legacy import-order?
Posted by Mark Liu <ma...@google.com.INVALID>.
+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 <am...@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 <kl...@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 <
> aljoscha@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é <jb...@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
> > > > > > <ki...@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
> > > > > <dh...@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 <
> > > > jesse@smokinghand.com
> > > > > >
> > > > > >>> wrote:
> > > > > >>>
> > > > > >>>> Please. That's the one that always trips me up.
> > > > > >>>>
> > > > > >>>> On Tue, Aug 23, 2016, 4:10 PM Ben Chambers <
> > bchambers@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é
> > > > > jbonofre@apache.org
> > > > > http://blog.nanthrax.net
> > > > > Talend - http://www.talend.com
> > > > >
> > > >
> > >
> >
>
Re: Remove legacy import-order?
Posted by Lukasz Cwik <lc...@google.com.INVALID>.
+1 for import order
On Wed, Aug 24, 2016 at 9:01 AM, Amit Sela <am...@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 <kl...@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 <al...@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é <jb...@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
> > > > > <ki...@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
> > > > <dh...@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 <
> > > jesse@smokinghand.com
> > > > >
> > > > >>> wrote:
> > > > >>>
> > > > >>>> Please. That's the one that always trips me up.
> > > > >>>>
> > > > >>>> On Tue, Aug 23, 2016, 4:10 PM Ben Chambers <
> bchambers@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é
> > > > jbonofre@apache.org
> > > > http://blog.nanthrax.net
> > > > Talend - http://www.talend.com
> > > >
> > >
> >
>
Re: Remove legacy import-order?
Posted by Amit Sela <am...@gmail.com>.
+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 <kl...@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 <al...@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é <jb...@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
> > > > <ki...@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
> > > <dh...@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 <
> > jesse@smokinghand.com
> > > >
> > > >>> wrote:
> > > >>>
> > > >>>> Please. That's the one that always trips me up.
> > > >>>>
> > > >>>> On Tue, Aug 23, 2016, 4:10 PM Ben Chambers <bc...@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é
> > > jbonofre@apache.org
> > > http://blog.nanthrax.net
> > > Talend - http://www.talend.com
> > >
> >
>
Re: Remove legacy import-order?
Posted by Kenneth Knowles <kl...@google.com.INVALID>.
+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 <al...@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é <jb...@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
> > > <ki...@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
> > <dh...@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 <
> jesse@smokinghand.com
> > >
> > >>> wrote:
> > >>>
> > >>>> Please. That's the one that always trips me up.
> > >>>>
> > >>>> On Tue, Aug 23, 2016, 4:10 PM Ben Chambers <bc...@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é
> > jbonofre@apache.org
> > http://blog.nanthrax.net
> > Talend - http://www.talend.com
> >
>
Re: Remove legacy import-order?
Posted by Aljoscha Krettek <al...@apache.org>.
+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é <jb...@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
> > <ki...@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
> <dh...@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 <jesse@smokinghand.com
> >
> >>> wrote:
> >>>
> >>>> Please. That's the one that always trips me up.
> >>>>
> >>>> On Tue, Aug 23, 2016, 4:10 PM Ben Chambers <bc...@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é
> jbonofre@apache.org
> http://blog.nanthrax.net
> Talend - http://www.talend.com
>
Re: Remove legacy import-order?
Posted by Jean-Baptiste Onofré <jb...@nanthrax.net>.
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
> <ki...@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 <dh...@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 <bc...@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�
jbonofre@apache.org
http://blog.nanthrax.net
Talend - http://www.talend.com
Re: Remove legacy import-order?
Posted by Ben Chambers <bc...@google.com.INVALID>.
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
<ki...@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 <dh...@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 <bc...@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
> > > >
> > >
> >
>
Re: Remove legacy import-order?
Posted by Eugene Kirpichov <ki...@google.com.INVALID>.
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 <dh...@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 <bc...@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
> > >
> >
>
Re: Remove legacy import-order?
Posted by Dan Halperin <dh...@google.com.INVALID>.
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 <bc...@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
> >
>
Re: Remove legacy import-order?
Posted by Jesse Anderson <je...@smokinghand.com>.
Please. That's the one that always trips me up.
On Tue, Aug 23, 2016, 4:10 PM Ben Chambers <bc...@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
>