-
Notifications
You must be signed in to change notification settings - Fork 129
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
[CODE STYLE] Switch checkstyle to spotless #297
Conversation
Switches checkstyle functionality to spotless. Sets ratchetFrom to origin/main. This will mean it will only fail on files recently changed. Pull formatting from OpenSearch. Signed-off-by: John Mazanec <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #297 +/- ##
=========================================
Coverage 83.38% 83.38%
Complexity 885 885
=========================================
Files 127 127
Lines 3834 3834
Branches 361 361
=========================================
Hits 3197 3197
Misses 475 475
Partials 162 162 Continue to review full report at Codecov.
|
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.
LGTM! Thanks
@@ -37,7 +37,7 @@ plugins { | |||
id 'nebula.ospackage' version "8.3.0" apply false | |||
id 'idea' | |||
id 'jacoco' | |||
id 'checkstyle' | |||
id "com.diffplug.spotless" version "6.3.0" apply false |
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.
do we need to align with OpenSearch in versions? They are using 6.2, not sure if there is much of a difference.
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.
6.2 fails a log4j check, so I bumped to 6.3. Dont think it should be any issue.
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.
agree, thanks for clarifying
Overall @martin-gaievski @VijayanB, what do you think about ratchetFrom strategy? Is it better to just fix everything in one sweep or do it incrementally? |
May be we can keep this PR and create next PR with all formatting changes and remove ratchet from third pr |
I used ratchetFrom strategy before, it proved to work well. I think we can start with it, and plan to fix everything in one shot in future. |
That makes sense to me @martin-gaievski. |
Switches checkstyle functionality to spotless. Sets ratchetFrom to origin/main. This will mean it will only fail on files recently changed. Pull formatting from OpenSearch. Signed-off-by: John Mazanec <[email protected]> (cherry picked from commit 0db9b2e)
Switches checkstyle functionality to spotless. Sets ratchetFrom to origin/main. This will mean it will only fail on files recently changed. Pull formatting from OpenSearch. Signed-off-by: John Mazanec <[email protected]>
Description
Switches checkstyle functionality to spotless. Sets ratchetFrom to origin/main. This will mean it will only fail on files recently changed. Formatting guidelines were pulled from OpenSearch directly: https://github.com/opensearch-project/OpenSearch/blob/main/buildSrc/formatterConfig.xml.
Developer guide has been updated.
Checkstyle dependencies were removed.
Planning on backporting to 1.x and changing the branch in the ratchetFrom to 1.x.
Issues Resolved
#293
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.