Adding back checkstyle for Java import order
59 views
Skip to first unread message
Chongsun Ahn
Mar 8, 2018, 12:13:13 AM3/8/18
to OpenLMIS Dev
Hey all,
Shalom,
Chongsun
--
There are 10 kinds of people in this world; those who understand binary, and those who don’t.
So a while back, early on in the v3 project, we added checkstyle to our microservice build process, which included rules for import order in Java code. You can see the post here (https://groups.google.com/forum/#!topic/openlmis-dev/CCwBglBFbpk).
A while later, I believe we decided to turn off the rule for Java import order (https://groups.google.com/forum/#!topic/openlmis-dev/Iu5P5sa3J-k) because back then, Google’s
Java Style Guide had esoteric and difficult to configure import guidelines.
Well it looks like Google updated their Style Guide to simplify import order to just static imports, then non-static imports, and ASCII sort order within each section. See here (https://google.github.io/styleguide/javaguide.html#s3.3-import-statements).
I think we should re-enable checking for import order in checkstyle. Reasons why:
- In our style guide (https://github.com/OpenLMIS/openlmis-template-service/blob/master/STYLE-GUIDE.md), we state that we have adopted Google’s Java Style Guide, so we should. Developers should not be surprised by code that does not follow the style guide, nor should they have to guess at which parts have been adopted or not adopted, once they realize we have not fully adopted it.
- Google’s Style Guide import order has simplified enough that it should be relatively easy to configure, particularly in IntelliJ. Instructions on how to configure IntelliJ for optimizing imports below.
- Because we don’t enforce import order in checkstyle while developing and building, many of our Java classes have all kinds of import orders, which will get automatically re-ordered when a different developer is working on the file and IntelliJ optimizes on the fly. This creates additional diffs in commits and code reviews, making it harder to see what was actually changed.
To configure IntelliJ to help with imports, you can get the IntelliJ Google Java XML, and import it (under Editor -> Code Style -> Java). The official XML is found online (https://github.com/google/styleguide/blob/gh-pages/intellij-java-google-style.xml).
Once it is imported, you can reformat code and optimize imports under the Code menu.
I know this will create some extra work once we enforce it in the build, but I think it should be relatively quick. Thoughts?
Shalom,
Chongsun
--
There are 10 kinds of people in this world; those who understand binary, and those who don’t.
Chongsun Ahn | chongs...@villagereach.org
Software Development Engineer
2900 Eastlake Ave. E, Suite 230, Seattle, WA 98102, USA
DIRECT: 1.206.512.1536 CELL: 1.206.910.0973 FAX: 1.206.860.6972
SKYPE: chongsun.ahn.vr
Łukasz Lewczyński
Mar 8, 2018, 3:26:29 AM3/8/18
to Chongsun Ahn, OpenLMIS Dev
If we have state in our STYLE GUIDE that we adopt Google’s Java Style Guide then we should do it and there should not be discussion to not do it. Can we somehow use checkstyle/PMD to ensure that imports are in correct order?
On Thu, Mar 8, 2018 at 6:13 AM, Chongsun Ahn <chongs...@villagereach.org> wrote:
Hey all,
So a while back, early on in the v3 project, we added checkstyle to our microservice build process, which included rules for import order in Java code. You can see the post here (https://groups.google.com/forum/#!topic/openlmis-dev/CCwBglBFbpk). A while later, I believe we decided to turn off the rule for Java import order (https://groups.google.com/forum/#!topic/openlmis-dev/Iu5P5sa3J-k) because back then, Google’s Java Style Guide had esoteric and difficult to configure import guidelines.
Well it looks like Google updated their Style Guide to simplify import order to just static imports, then non-static imports, and ASCII sort order within each section. See here (https://google.github.io/styleguide/javaguide.html#s3.3-import-statements).
I think we should re-enable checking for import order in checkstyle. Reasons why:
- In our style guide (https://github.com/OpenLMIS/openlmis-template-service/blob/master/STYLE-GUIDE.md), we state that we have adopted Google’s Java Style Guide, so we should. Developers should not be surprised by code that does not follow the style guide, nor should they have to guess at which parts have been adopted or not adopted, once they realize we have not fully adopted it.
- Google’s Style Guide import order has simplified enough that it should be relatively easy to configure, particularly in IntelliJ. Instructions on how to configure IntelliJ for optimizing imports below.
- Because we don’t enforce import order in checkstyle while developing and building, many of our Java classes have all kinds of import orders, which will get automatically re-ordered when a different developer is working on the file and IntelliJ optimizes on the fly. This creates additional diffs in commits and code reviews, making it harder to see what was actually changed.
To configure IntelliJ to help with imports, you can get the IntelliJ Google Java XML, and import it (under Editor -> Code Style -> Java). The official XML is found online (https://github.com/google/styleguide/blob/gh-pages/intellij-java-google-style.xml). Once it is imported, you can reformat code and optimize imports under the Code menu.
I know this will create some extra work once we enforce it in the build, but I think it should be relatively quick. Thoughts?
Shalom,
Chongsun
--
There are 10 kinds of people in this world; those who understand binary, and those who don’t.
Chongsun Ahn | chongsun.ahn@villagereach.org
Software Development EngineerDIRECT: 1.206.512.1536 CELL: 1.206.910.0973 FAX: 1.206.860.6972SKYPE: chongsun.ahn.vr
--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/F7448458-6789-42A0-8C05-2FF1B3E0CB13%40villagereach.org.
For more options, visit https://groups.google.com/d/optout.
SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41
Paweł Albecki
Mar 8, 2018, 5:07:26 AM3/8/18
to Łukasz Lewczyński, Chongsun Ahn, OpenLMIS Dev
Hi,
I'm happy that we will re-enable these checks. The Google Style Guide XML has also some rules about blank lines etc. that our Java classes have all kinds of too. It shouldn't be much extra work for developers to incorporate styleguide in IDE and it will result in faster reviews and better commit diffs.
Regards
Paweł
On Thu, Mar 8, 2018 at 9:26 AM, Łukasz Lewczyński <llewc...@soldevelo.com> wrote:
If we have state in our STYLE GUIDE that we adopt Google’s Java Style Guide then we should do it and there should not be discussion to not do it. Can we somehow use checkstyle/PMD to ensure that imports are in correct order?
On Thu, Mar 8, 2018 at 6:13 AM, Chongsun Ahn <chongs...@villagereach.org> wrote:
Hey all,
So a while back, early on in the v3 project, we added checkstyle to our microservice build process, which included rules for import order in Java code. You can see the post here (https://groups.google.com/forum/#!topic/openlmis-dev/CCwBglBFbpk). A while later, I believe we decided to turn off the rule for Java import order (https://groups.google.com/forum/#!topic/openlmis-dev/Iu5P5sa3J-k) because back then, Google’s Java Style Guide had esoteric and difficult to configure import guidelines.
Well it looks like Google updated their Style Guide to simplify import order to just static imports, then non-static imports, and ASCII sort order within each section. See here (https://google.github.io/styleguide/javaguide.html#s3.3-import-statements).
I think we should re-enable checking for import order in checkstyle. Reasons why:
- In our style guide (https://github.com/OpenLMIS/openlmis-template-service/blob/master/STYLE-GUIDE.md), we state that we have adopted Google’s Java Style Guide, so we should. Developers should not be surprised by code that does not follow the style guide, nor should they have to guess at which parts have been adopted or not adopted, once they realize we have not fully adopted it.
- Google’s Style Guide import order has simplified enough that it should be relatively easy to configure, particularly in IntelliJ. Instructions on how to configure IntelliJ for optimizing imports below.
- Because we don’t enforce import order in checkstyle while developing and building, many of our Java classes have all kinds of import orders, which will get automatically re-ordered when a different developer is working on the file and IntelliJ optimizes on the fly. This creates additional diffs in commits and code reviews, making it harder to see what was actually changed.
To configure IntelliJ to help with imports, you can get the IntelliJ Google Java XML, and import it (under Editor -> Code Style -> Java). The official XML is found online (https://github.com/google/styleguide/blob/gh-pages/intellij-java-google-style.xml). Once it is imported, you can reformat code and optimize imports under the Code menu.
I know this will create some extra work once we enforce it in the build, but I think it should be relatively quick. Thoughts?
Shalom,
Chongsun
--
There are 10 kinds of people in this world; those who understand binary, and those who don’t.
Chongsun Ahn | chongs...@villagereach.org
Software Development EngineerDIRECT: 1.206.512.1536 CELL: 1.206.910.0973 FAX: 1.206.860.6972SKYPE: chongsun.ahn.vr
--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/F7448458-6789-42A0-8C05-2FF1B3E0CB13%40villagereach.org.
For more options, visit https://groups.google.com/d/optout.
SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41
--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CAAdp53yR5F0xLBsYY%2BZDbLkr0RjS26F-%2B0JYV-O_guYY2jfxuA%40mail.gmail.com.
Chongsun Ahn
Mar 13, 2018, 4:27:19 PM3/13/18
to OpenLMIS Dev, Paweł Albecki, Łukasz Lewczyński
Hey everyone,
Yes, we can have checkstyle ensuring imports are in the correct order. I have created OLMIS-4295 for this work and added it for backlog grooming.
Shalom,
Chongsun
--
There are 10 kinds of people in this world; those who understand binary, and those who don’t.
Chongsun Ahn | chongs...@villagereach.org
Software Development Engineer
2900 Eastlake Ave. E, Suite 230, Seattle, WA 98102, USA
DIRECT: 1.206.512.1536 CELL: 1.206.910.0973 FAX: 1.206.860.6972
SKYPE: chongsun.ahn.vr
Reply all
Reply to author
Forward
0 new messages