-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Spotless] Applying Google Code Format for core/src/main files #1 #312
[Spotless] Applying Google Code Format for core/src/main files #1 #312
Conversation
Codecov Report
@@ Coverage Diff @@
## integ/sl_GoogleJavaFormat1 #312 +/- ##
=============================================================
Coverage ? 97.42%
Complexity ? 4646
=============================================================
Files ? 408
Lines ? 11550
Branches ? 838
=============================================================
Hits ? 11253
Misses ? 290
Partials ? 7
Flags with carried forward coverage won't be shown. Click here to find out more. 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
cd0826a
to
25bfe09
Compare
* LogicalProject[Ref("abs(age)"), sub(Ref("sum(age)"), Ref("avg(age)")) | ||
* LogicalAgg(agg=[sum(age), avg(age)], group=[abs(age)]] | ||
* LogicalRelation | ||
* The optimizer used to replace the expression referred in the SelectClause e.g. The query SELECT |
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.
This javadoc loses info with merging lines. Probably you need to add javadoc tags (pre
I suppose) to keep it visual and readable.
Please, go though all changed multiline javadocs to review.
c1174c4
to
60c0018
Compare
03127d0
to
613535f
Compare
core/src/main/java/org/opensearch/sql/analysis/SelectExpressionAnalyzer.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/opensearch/sql/data/type/WideningTypeRule.java
Outdated
Show resolved
Hide resolved
7d55f5a
to
d04dd9f
Compare
* [WHEN compare_expr THEN result_expr] ... | ||
* [ELSE result_expr] | ||
* END | ||
* CASE case_value_expr WHEN compare_expr THEN result_expr [WHEN compare_expr THEN result_expr] |
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.
Also needs <pre>
tag
* NULL MISSING FALSE | ||
* MISSING NULL FALSE | ||
* MISSING MISSING TRUE | ||
* The customize equals logic. The table below list the NULL and MISSING handling logic. A B A == |
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.
Also needs <pre>
tag
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.
Addressed by adding table in d455533
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.
@MitchellGale There's one of Yury's commits in here. Would you be able to rebase and remove that commit?
54cb07d
to
a43d97d
Compare
import java.util.List; | ||
import java.util.Set; |
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.
Cleaning imports is a part of another check. Do you want to do it first?
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.
It's on the agenda for being put into upstream, I don't think it'll cause a merge conflict. Need to wait for opensearch-project#1924 to be merged then I can put up the remove unused imports PR up.
when.getResult() | ||
), context)); | ||
whens.add( | ||
(WhenClause) |
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.
really?
* LogicalPlan should be<br> | ||
* LogicalProject[Ref("abs(age)"), sub(Ref("sum(age)"), Ref("avg(age)"))<br> | ||
* &ensp LogicalAgg(agg=[sum(age), avg(age)], group=[abs(age)]]<br> | ||
* &emsp LogicalRelation |
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.
Is it better than using pre
? Or GJF ignores pre
and rewrites the comment?
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.
It's properly formatted this way vs using pre tag and just asking it not to be formatted. Both work.
* Agg(Alias("AVG(age)", aggExpr), Alias("length(name)", groupExpr)) | ||
* </pre> | ||
* The Alias could be<br> | ||
* 1. SELECT name, AVG(age) FROM s BY name -><br> |
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.
Use ul
and li
HTML tags for list formatting.
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.
Fixed
@ToString | ||
@EqualsAndHashCode(callSuper = false) | ||
public class AllFields extends UnresolvedExpression { | ||
public static final AllFields INSTANCE = new AllFields(); | ||
|
||
private AllFields() { | ||
} | ||
private AllFields() {} |
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.
This conflicts with checkstyle, but I like it more.
Consider updating checkstyle rules in future.
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.
Checkstyle will be removed in favour of spotless. Currently it is set to "ignore test failures".
throw new SemanticCheckException(String.format("date:%s in unsupported format, please use " | ||
+ "yyyy-MM-dd", date)); | ||
throw new SemanticCheckException( | ||
String.format("date:%s in unsupported format, please use " + "yyyy-MM-dd", date)); |
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.
String.format("date:%s in unsupported format, please use " + "yyyy-MM-dd", date)); | |
String.format("date: %s in unsupported format, please use 'yyyy-MM-dd'", date)); |
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.
Probably you need to update test if you apply this change.
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.
Partly resolved in da35a7e. (no additional quotes)
+ "use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]", datetime)); | ||
throw new SemanticCheckException( | ||
String.format( | ||
"datetime:%s in unsupported format, please " + "use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]", |
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.
"datetime:%s in unsupported format, please " + "use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]", | |
"datetime: %s in unsupported format, please 'use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]'", |
Could be followed by test update
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 want the single quotes around yyyy-MM-dd instead?
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.
Oh right
"datetime:%s in unsupported format, please " | ||
+ "use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]", |
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.
"datetime:%s in unsupported format, please " | |
+ "use yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]", | |
"datetime: %s in unsupported format, please use 'yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]'", |
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.
Resolved.
@@ -34,6 +34,7 @@ | |||
import lombok.SneakyThrows; | |||
import org.apache.commons.lang3.reflect.FieldUtils; | |||
import org.apache.lucene.search.TotalHits; | |||
import org.junit.jupiter.api.BeforeAll; |
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.
why?
import org.junit.jupiter.api.BeforeAll; |
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.
Thanks! Added as a part of an old fix I was testing... missed removing the import.
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.
removed.
d6b6aab
to
0fae29c
Compare
7a52c9b
to
d2de909
Compare
# This is the 1st commit message: Fix create_index/create_index_with_IOException issue caused by OpenSearch PR change (opensearch-project#1899) * Added setDefaultMediaType for create_index and create_index_with_IOException Signed-off-by: Mitchell Gale <[email protected]> (cherry picked from commit 7b932a7) Signed-off-by: Mitchell Gale <[email protected]> Resolving merge conflicts for pre tag in java docs. Signed-off-by: Mitchell Gale <[email protected]> running spotless check on newly pre tagged javadocs. Signed-off-by: Mitchell Gale <[email protected]> Converts java doc table to proper java doc table. Signed-off-by: Mitchell Gale <[email protected]> Resolving merge conflicts for pre tag in java docs 2 Signed-off-by: Mitchell Gale <[email protected]> running spotless check on newly pre tagged javadocs. Signed-off-by: Mitchell Gale <[email protected]> cherry pick 60c0018 Signed-off-by: Mitchell Gale <[email protected]> Addressed PR comment for table format in AbstractExprValue. Signed-off-by: Mitchell Gale <[email protected]> Removed pre tag from ExpressionReferenceOptimizer. Signed-off-by: Mitchell Gale <[email protected]> Removed pre tag from AstDSL.java Signed-off-by: Mitchell Gale <[email protected]> Removed pre tag from AstDSL.java Signed-off-by: Mitchell Gale <[email protected]> Removed pre tag from SelectExpressionAnalyzer.java Signed-off-by: Mitchell Gale <[email protected]> fixed java doc in QualifiedName.java Signed-off-by: Mitchell Gale <[email protected]> Removing checkstyle test for core and added spotless for relevant directories. Signed-off-by: Mitchell Gale <[email protected]> # This is the commit message #2: Fixing spacing around headers in ExpressionReferenceOptimizer.java SelectExpressionAnalyzer.java Signed-off-by: Mitchell Gale <[email protected]> # This is the commit message #3: Fix breaking changes. Disable some flaky tests in legacy. Signed-off-by: Yury-Fridlyand <[email protected]> (cherry picked from commit 809e656) Signed-off-by: Mitchell Gale <[email protected]> # This is the commit message #4: Resolving merge conflicts for pre tag in java docs. Signed-off-by: Mitchell Gale <[email protected]> # This is the commit message #5: running spotless check on newly pre tagged javadocs. Signed-off-by: Mitchell Gale <[email protected]> # This is the commit message #6: Fixed java doc spelling and improving string concatination. Signed-off-by: Mitchell Gale <[email protected]> # This is the commit message #7: improving string concatination. Signed-off-by: Mitchell Gale <[email protected]> # This is the commit message #8: Improving failure format on some functions. Signed-off-by: Mitchell Gale <[email protected]> # This is the commit message #9: spotless apply and fix of build.gradle Signed-off-by: Mitchell Gale <[email protected]>
author Mitchell Gale <[email protected]> 1691080710 -0700 committer Mitchell Gale <[email protected]> 1691080766 -0700 Fix create_index/create_index_with_IOException issue caused by OpenSearch PR change (opensearch-project#1899) * Added setDefaultMediaType for create_index and create_index_with_IOException Signed-off-by: Mitchell Gale <[email protected]> (cherry picked from commit 7b932a7) Signed-off-by: Mitchell Gale <[email protected]> Resolving merge conflicts for pre tag in java docs. Signed-off-by: Mitchell Gale <[email protected]> running spotless check on newly pre tagged javadocs. Signed-off-by: Mitchell Gale <[email protected]> Converts java doc table to proper java doc table. Signed-off-by: Mitchell Gale <[email protected]> Resolving merge conflicts for pre tag in java docs 2 Signed-off-by: Mitchell Gale <[email protected]> running spotless check on newly pre tagged javadocs. Signed-off-by: Mitchell Gale <[email protected]> cherry pick 60c0018 Signed-off-by: Mitchell Gale <[email protected]> Addressed PR comment for table format in AbstractExprValue. Signed-off-by: Mitchell Gale <[email protected]> Removed pre tag from ExpressionReferenceOptimizer. Signed-off-by: Mitchell Gale <[email protected]> Removed pre tag from AstDSL.java Signed-off-by: Mitchell Gale <[email protected]> Removed pre tag from AstDSL.java Signed-off-by: Mitchell Gale <[email protected]> Removed pre tag from SelectExpressionAnalyzer.java Signed-off-by: Mitchell Gale <[email protected]> fixed java doc in QualifiedName.java Signed-off-by: Mitchell Gale <[email protected]> Removing checkstyle test for core and added spotless for relevant directories. Signed-off-by: Mitchell Gale <[email protected]> Fixing spacing around headers in ExpressionReferenceOptimizer.java SelectExpressionAnalyzer.java Signed-off-by: Mitchell Gale <[email protected]> Fix breaking changes. Disable some flaky tests in legacy. Signed-off-by: Yury-Fridlyand <[email protected]> (cherry picked from commit 809e656) Signed-off-by: Mitchell Gale <[email protected]> Resolving merge conflicts for pre tag in java docs. Signed-off-by: Mitchell Gale <[email protected]> running spotless check on newly pre tagged javadocs. Signed-off-by: Mitchell Gale <[email protected]> Fixed java doc spelling and improving string concatination. Signed-off-by: Mitchell Gale <[email protected]> improving string concatination. Signed-off-by: Mitchell Gale <[email protected]> Improving failure format on some functions. Signed-off-by: Mitchell Gale <[email protected]> spotless apply and fix of build.gradle Signed-off-by: Mitchell Gale <[email protected]> Resolving merge conflicts for pre tag in java docs. Signed-off-by: Mitchell Gale <[email protected]> running spotless check on newly pre tagged javadocs. Signed-off-by: Mitchell Gale <[email protected]> Converts java doc table to proper java doc table. Signed-off-by: Mitchell Gale <[email protected]> Removing unused import in OpenSearchRestClientTest.java Signed-off-by: Mitchell Gale <[email protected]>
* The Alias could be<br> | ||
* | ||
* <ol> | ||
* <li>ELECT name, AVG(age) FROM s BY name -><br> |
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.
SELECT
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.
Fixed, ty
48df392
to
7579925
Compare
Signed-off-by: Mitchell Gale <[email protected]>
Description
This PR applies Spotless on Java code for:
core/src/main/java/org/opensearch/sql/analysis
core/src/main/java/org/opensearch/sql/ast
core/src/main/java/org/opensearch/sql/data
core/src/main/java/org/opensearch/sql/datasource
Issues Resolved
[List any issues this PR will resolve]
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.