-
Notifications
You must be signed in to change notification settings - Fork 29
Verify Java imports are sorted #5853
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #5853 +/- ##
=============================================
- Coverage 74.48% 74.45% -0.03%
Complexity 5247 5247
=============================================
Files 366 366
Lines 18974 18974
Branches 2020 2020
=============================================
- Hits 14132 14127 -5
- Misses 3886 3890 +4
- Partials 956 957 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
name: Check Java imports sorted | ||
# A dirty Java file after a build probably means it was committed without building, and its imports are out of order | ||
command: | | ||
if [[ $(git diff --name-only | grep "\.java$") != '' ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To my surprise, although it makes sense when you think about it, there were other files showing up as modified -- the decrypted migrations files. So, look for modified Java files only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forget, do we have a separate check for the openapi.yaml?
Does it make sense to put these two together? (I don't see it in a superficial scan, was it unstable?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, I added the check for openapi.yaml.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear, I don't remember the story behind this (i.e. I would have though we would have had it somewhere else before, but maybe we removed it), but worth a try
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious about story
Can you please elaborate? |
|
name: Check Java imports sorted | ||
# A dirty Java file after a build probably means it was committed without building, and its imports are out of order | ||
command: | | ||
if [[ $(git diff --name-only | grep "\.java$") != '' ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear, I don't remember the story behind this (i.e. I would have though we would have had it somewhere else before, but maybe we removed it), but worth a try
Description
Background
The Maven impsort plugin sorts Java imports as part of the build. If you commit a Java file without having done a build, imports may be out of order.
Fix
After a Maven build on CircleCI, check that there are no modified Java files in the Git tree. A modified Java file indicates that the impsort plugin only ran on CircleCI, and wasn't run before the file was committed.
For a CircleCI example of a failing build because imports weren't sorted, see here.
Review Instructions
Issue
SEAB-3217
Security and Privacy
If there are any concerns that require extra attention from the security team, highlight them here and check the box when complete.
e.g. Does this change...
Please make sure that you've checked the following before submitting your pull request. Thanks!
mvn clean install
@RolesAllowed
annotation