11:55 a.m.
Hi,
Is there an agreement on the order of import statements in our codebase?
I'm using eclipse which, by default, organize imports in the following order:
static
java
javax
org
com
but we have some files with a different order *cough*intellij*cough* :)
That makes for some noise in the commits when I modify a class and eclipse changes the
imports order.
If we agree with this default order, I can add a checkstyle rule[1] for this.
wdyt?
jeff
[1] http://checkstyle.sourceforge.net/config_imports.html#ImportOrder
--
Jeff Mesnil
JBoss, a division of Red Hat
http://jmesnil.net/
2:53 p.m.
On 10/09/2012 11:55 AM, Jeff Mesnil wrote:
Hi,
Is there an agreement on the order of import statements in our codebase?
I'm using eclipse which, by default, organize imports in the following order:
static
java
javax
org
com
but we have some files with a different order *cough*intellij*cough* :)
That makes for some noise in the commits when I modify a class and eclipse changes the
imports order.
If we agree with this default order, I can add a checkstyle rule[1] for this.
We tried this before but it didn't work out because checkstyle gets
confused about imports which are only used in Javadoc, and because
regardless of checkstyle rules everyone cannot agree on an order, and
anyway changes to existing code should NEVER EVER EVER EVER include
import "cleanups", "reorgs", or anything of the sort (no matter how
bad
you think it is) along with "real" changes because it just obscures what
has changed and causes trouble. The same goes for whitespace fixes. If
a file is very offensively formatted, I could see considering a
specific, dedicated commit whose sole purpose and content is to clean up
the whitespace (i.e. absolutely no functional changes), but other than
that... no.
For new code I can't imagine it really matters as long as you don't use
star imports (though using star with static import for certain classes
is just fine).
--
- DML
4:29 p.m.
On Tue, Oct 9, 2012 at 9:53 PM, David M. Lloyd <david.lloyd(a)redhat.com>wrote:
On 10/09/2012 11:55 AM, Jeff Mesnil wrote:
> Hi,
>
> Is there an agreement on the order of import statements in our codebase?
>
> I'm using eclipse which, by default, organize imports in the following
order:
>
> static
>
> java
> javax
> org
> com
>
> but we have some files with a different order *cough*intellij*cough* :)
> That makes for some noise in the commits when I modify a class and
eclipse changes the imports order.
>
> If we agree with this default order, I can add a checkstyle rule[1] for
this.
We tried this before but it didn't work out because checkstyle gets
confused about imports which are only used in Javadoc, and because
regardless of checkstyle rules everyone cannot agree on an order, and
anyway changes to existing code should NEVER EVER EVER EVER include
import "cleanups", "reorgs", or anything of the sort (no matter how
bad
you think it is) along with "real" changes because it just obscures what
has changed and causes trouble. The same goes for whitespace fixes. If
a file is very offensively formatted, I could see considering a
specific, dedicated commit whose sole purpose and content is to clean up
the whitespace (i.e. absolutely no functional changes), but other than
that... no.
Jeff, i generally agree with you and can share my idea code-style settings
with
you, but David has a point :|
I just enabled "only text changed to vcs" when reformatting code, so you it
will be reformatting just changed code and noting else.
For new code I can't imagine it really matters as long as you
don't use
star imports (though using star with static import for certain classes
is just fine).
checkstyle checks for this during build, so no worries about abusing this.
--
- DML
_______________________________________________
jboss-as7-dev mailing list
jboss-as7-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
--
Tomaz
12:16 p.m.
On 10/19/2012 08:38 AM, Jaikiran Pai wrote:
On Wednesday 10 October 2012 02:59 AM, Tomaž Cerar wrote:
> I just enabled "only text changed to vcs" when reformatting code, so
> you it will be reformatting just changed code and noting else.
Where in IntelliJ IDEA can I set this?
When you see the dialog box that pops up
when you reformat, there is a
check box for it there.
-Jaikiran
_______________________________________________
jboss-as7-dev mailing list
jboss-as7-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
--
James R. Perkins
JBoss by Red Hat
12:28 p.m.
Here is a picture that I got from Brian a while back.
http://dl.dropbox.com/u/35343318/intellij_importhandling.png
On 10/19/2012 11:38 AM, Jaikiran Pai wrote:
On Wednesday 10 October 2012 02:59 AM, Tomaž Cerar wrote:
>
> I just enabled "only text changed to vcs" when reformatting code, so
> you it will be reformatting just changed code and noting else.
Where in IntelliJ IDEA can I set this?
-Jaikiran
_______________________________________________
jboss-as7-dev mailing list
jboss-as7-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
1:51 a.m.
On 10/09/2012 09:53 PM, David M. Lloyd wrote:
On 10/09/2012 11:55 AM, Jeff Mesnil wrote:
> If we agree with this default order, I can add a checkstyle rule[1] for this.
We tried this before but it didn't work out because checkstyle gets
confused about imports which are only used in Javadoc, and because
regardless of checkstyle rules everyone cannot agree on an order, and
anyway changes to existing code should NEVER EVER EVER EVER include
import "cleanups", "reorgs", or anything of the sort (no matter how
bad
you think it is) along with "real" changes because it just obscures what
has changed and causes trouble.
I agree with you and my point was to avoid these cosmetic changes as much as possible.
Then the onus is on me to not rely on the IDE to build the package imports and check
manually in case I added by error unwanted
packages.
--
Jeff Mesnil
JBoss, a division of Red Hat
http://jmesnil.net/
5:06 a.m.
On 10/09/2012 09:53 PM, David M. Lloyd wrote:
If
a file is very offensively formatted, I could see considering a
specific, dedicated commit whose sole purpose and content is to clean up
the whitespace (i.e. absolutely no functional changes), but other than
that... no.
Even then I would reconsider it, because it makes for a hell should
cherry-picking or history analysis be needed.
Carlo
9:46 a.m.
I do agree that we should avoid lots of reordering, but IMO one thing that would help (to
Jeff's point) is if we did have an official standard for it.
On Oct 10, 2012, at 5:06 AM, Carlo de Wolf <cdewolf(a)redhat.com> wrote:
On 10/09/2012 09:53 PM, David M. Lloyd wrote:
> If
> a file is very offensively formatted, I could see considering a
> specific, dedicated commit whose sole purpose and content is to clean up
> the whitespace (i.e. absolutely no functional changes), but other than
> that... no.
Even then I would reconsider it, because it makes for a hell should
cherry-picking or history analysis be needed.
Carlo
_______________________________________________
jboss-as7-dev mailing list
jboss-as7-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
10:25 a.m.
+1.
And I know a year or so ago a number of IDEA users tweaked their configs
to follow the eclipse standard Jeff listed, so my vote is this becomes
the standard.
On 10/10/12 9:46 AM, Jason Greene wrote:
I do agree that we should avoid lots of reordering, but IMO one thing
that would help (to Jeff's point) is if we did have an official standard for it.
On Oct 10, 2012, at 5:06 AM, Carlo de Wolf <cdewolf(a)redhat.com> wrote:
> On 10/09/2012 09:53 PM, David M. Lloyd wrote:
>> If
>> a file is very offensively formatted, I could see considering a
>> specific, dedicated commit whose sole purpose and content is to clean up
>> the whitespace (i.e. absolutely no functional changes), but other than
>> that... no.
>
> Even then I would reconsider it, because it makes for a hell should
> cherry-picking or history analysis be needed.
>
> Carlo
> _______________________________________________
> jboss-as7-dev mailing list
> jboss-as7-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
_______________________________________________
jboss-as7-dev mailing list
jboss-as7-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
--
Brian Stansberry
Principal Software Engineer
JBoss by Red Hat
10:29 a.m.
I don't care about the package import order, but if there's going to be
a standard I'd strongly urge people to put static imports *after* the
package import block. And like I said earlier, package * imports are
bad, but static * imports are usually OK.
On 10/10/2012 10:25 AM, Brian Stansberry wrote:
+1.
And I know a year or so ago a number of IDEA users tweaked their configs
to follow the eclipse standard Jeff listed, so my vote is this becomes
the standard.
On 10/10/12 9:46 AM, Jason Greene wrote:
> I do agree that we should avoid lots of reordering, but IMO one thing that would help
(to Jeff's point) is if we did have an official standard for it.
>
> On Oct 10, 2012, at 5:06 AM, Carlo de Wolf <cdewolf(a)redhat.com> wrote:
>
>> On 10/09/2012 09:53 PM, David M. Lloyd wrote:
>>> If
>>> a file is very offensively formatted, I could see considering a
>>> specific, dedicated commit whose sole purpose and content is to clean up
>>> the whitespace (i.e. absolutely no functional changes), but other than
>>> that... no.
>>
>> Even then I would reconsider it, because it makes for a hell should
>> cherry-picking or history analysis be needed.
>>
>> Carlo
>> _______________________________________________
>> jboss-as7-dev mailing list
>> jboss-as7-dev(a)lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
>
>
> _______________________________________________
> jboss-as7-dev mailing list
> jboss-as7-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
>
--
- DML
9:58 p.m.
Yeah agreed. I think that having a standard way of fixing the import order list just makes
for easy reading. And sadly, in my case I know exactly where I have to manually put in
imports when my IDE fails to recognise something like java.util.List.
On the point earlier about checkstyles not being able to look at the import order I seem
to remember the checkstyle tool on Tattletale being able to look out for that - because
IntelliJ got the orders wrong. I don't know if Jesper is listening/watching but
perhaps he could shed some light on this?
----- Original Message -----
From: "Brian Stansberry" <brian.stansberry(a)redhat.com>
To: jboss-as7-dev(a)lists.jboss.org
Sent: Wednesday, October 10, 2012 11:25:13 PM
Subject: Re: [jboss-as7-dev] order of import statements
+1.
And I know a year or so ago a number of IDEA users tweaked their configs
to follow the eclipse standard Jeff listed, so my vote is this becomes
the standard.
On 10/10/12 9:46 AM, Jason Greene wrote:
I do agree that we should avoid lots of reordering, but IMO one thing
that would help (to Jeff's point) is if we did have an official standard for it.
On Oct 10, 2012, at 5:06 AM, Carlo de Wolf <cdewolf(a)redhat.com> wrote:
> On 10/09/2012 09:53 PM, David M. Lloyd wrote:
>> If
>> a file is very offensively formatted, I could see considering a
>> specific, dedicated commit whose sole purpose and content is to clean up
>> the whitespace (i.e. absolutely no functional changes), but other than
>> that... no.
>
> Even then I would reconsider it, because it makes for a hell should
> cherry-picking or history analysis be needed.
>
> Carlo
> _______________________________________________
> jboss-as7-dev mailing list
> jboss-as7-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
_______________________________________________
jboss-as7-dev mailing list
jboss-as7-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
--
Brian Stansberry
Principal Software Engineer
JBoss by Red Hat
_______________________________________________
jboss-as7-dev mailing list
jboss-as7-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/jboss-as7-dev
4518
days inactive
4528
days old
11 comments
10 participants
participants (10)
-
Brian Stansberry -
Carlo de Wolf -
David M. Lloyd -
Jaikiran Pai -
James Perkins -
Jason Greene -
Jeff Mesnil -
Navin Surtani -
Scott Marlow -
Tomaž Cerar