[SPARK-47929] Setup Static Analysis for Operator #6
Conversation
build.gradle
Outdated
| } | ||
| } | ||
|
|
||
| assert JavaVersion.current().isJava11Compatible(): "Java 11 or newer is required" |
There was a problem hiding this comment.
Please enforce Java 17 or newer
build.gradle
Outdated
| endWithNewline() | ||
| googleJavaFormat('1.17.0') | ||
| importOrder( | ||
| 'java', |
| --> | ||
|
|
||
| <module name="Checker"> |
There was a problem hiding this comment.
This is a copy of the following, isn't it? May I ask what is the difference and why you change that, @jiangzho ?
There was a problem hiding this comment.
Yes, this is taken from Spark repo. The major difference is to
- Remove
checkstyle-suppressions.xmlas there's no expected SuppressionFilter expected for this repository yet. - Style difference for line length / indent .etc
config/checkstyle/checkstyle.xml
Outdated
| --> | ||
|
|
||
| <module name="Checker"> | ||
| <property name="charset" value="UTF-8"/> |
There was a problem hiding this comment.
Although I know that this 4-space indentation inherits from Apache Spark's file, shall we use 2-space indentation because this is a new repository?
gradle.properties
Outdated
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
| # |
There was a problem hiding this comment.
Shall we remove this line?
gradle.properties
Outdated
| checkstyleVersion=10.15.0 | ||
| pmdVersion=6.55.0 | ||
| spotBugsGradlePluginVersion=6.0.12 | ||
| spotBugsVersion=4.8.3 |
There was a problem hiding this comment.
Could you make it sure that this PR proposes all latest versions? If there is non-latest versions, could you explain the reason?
config/pmd/ruleset.xml
Outdated
| --> | ||
|
|
||
| <ruleset name="pmd ruleset"> | ||
| <description> |
There was a problem hiding this comment.
ditto. Please use 2-space indentation.
build.gradle
Outdated
| configurations.all { | ||
| resolutionStrategy { | ||
| force "org.slf4j:slf4j-api:$slf4jVersion" | ||
| force "io.fabric8:kubernetes-model-core:$fabric8Version" |
There was a problem hiding this comment.
I believe these two lines are irrelevant to Static Analysis. Please remove the irrelevant ones.
config/lombok/lombok.config
Outdated
| config.stopBubbling = true | ||
| lombok.addLombokGeneratedAnnotation = true | ||
| lombok.extern.findbugs.addSuppressFBWarnings = true | ||
| lombok.anyConstructor.addConstructorProperties = true |
There was a problem hiding this comment.
- This file doesn't have ASF License.
- Can we eliminate this
Lombokdependency completely?
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
I review the first round review.
- Please address the review comments.
- Please respect the Apache Spark PR template like the following.
|
Thanks @dongjoon-hyun for the review! yes, we target latest versions as possible Checkstyle latest 10.15.0 |
dongjoon-hyun
changed the title
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
+1, LGTM. Thank you, @jiangzho .
Merged to main.
2 participants
What changes were proposed in this pull request?
This is a breakdown PR from #2 - setting up common build Java tasks and corresponding plugins.
Why are the changes needed?
This PR includes checkstyle, pmd, spotbugs. Also includes jacoco for coverage analysis, spotless for formatting. These tasks can help to enhance the quality of future Java contributions. They can also be referred in CI tasks for automation.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Tested manually.
Was this patch authored or co-authored using generative AI tooling?
no