You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@seatunnel.apache.org by GitBox <gi...@apache.org> on 2023/01/12 16:13:07 UTC
[GitHub] [incubator-seatunnel] FlechazoW opened a new pull request, #3939: [Improve][Code-Style] Add the spotless plugin for unified code style.
FlechazoW opened a new pull request, #3939:
URL: https://github.com/apache/incubator-seatunnel/pull/3939
<!--
Thank you for contributing to SeaTunnel! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
## Contribution Checklist
- Make sure that the pull request corresponds to a [GITHUB issue](https://github.com/apache/incubator-seatunnel/issues).
- Name the pull request in the form "[Feature] [component] Title of the pull request", where *Feature* can be replaced by `Hotfix`, `Bug`, etc.
- Minor fixes should be named following this pattern: `[hotfix] [docs] Fix typo in README.md doc`.
-->
## Purpose of this pull request
<!-- Describe the purpose of this pull request. For example: This pull request adds checkstyle plugin.-->
close #2492.
## Check list
* [ ] Code changed are covered with tests, or it does not need tests for reason:
* [ ] If any new Jar binary package adding in your PR, please add License Notice according
[New License Guide](https://github.com/apache/incubator-seatunnel/blob/dev/docs/en/contribution/new-license.md)
* [ ] If necessary, please update the documentation to describe the new feature. https://github.com/apache/incubator-seatunnel/tree/dev/docs
* [ ] If you are contributing the connector code, please check that the following files are updated:
1. Update change log that in connector document. For more details you can refer to [connector-v2](https://github.com/apache/incubator-seatunnel/tree/dev/docs/en/connector-v2)
2. Update [plugin-mapping.properties](https://github.com/apache/incubator-seatunnel/blob/dev/plugin-mapping.properties) and add new connector information in it
3. Update the pom file of [seatunnel-dist](https://github.com/apache/incubator-seatunnel/blob/dev/seatunnel-dist/pom.xml)
* [ ] Update the [`release-note`](https://github.com/apache/incubator-seatunnel/blob/dev/release-note.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] zhuangchong commented on pull request #3939: [Improve][Code-Style] Add the spotless plugin for unified code style.
Posted by "zhuangchong (via GitHub)" <gi...@apache.org>.
zhuangchong commented on PR #3939:
URL: https://github.com/apache/incubator-seatunnel/pull/3939#issuecomment-1408164678
Are spotless and checkstyle complementary? Has this been discussed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] zhuangchong commented on a diff in pull request #3939: [Improve][Code-Style] Add the spotless plugin for unified code style.
Posted by "zhuangchong (via GitHub)" <gi...@apache.org>.
zhuangchong commented on code in PR #3939:
URL: https://github.com/apache/incubator-seatunnel/pull/3939#discussion_r1090289761
##########
.github/workflows/backend.yml:
##########
@@ -302,14 +301,14 @@ jobs:
- name: run all modules unit test
if: needs.changes.outputs.api == 'true'
run: |
- ./mvnw -B -T 1C clean verify -D"maven.test.skip"=false -D"checkstyle.skip"=true -D"license.skipAddThirdParty"=true --no-snapshot-updates
+ ./mvnw -B -T 1C clean verify -D"maven.test.skip"=false -D"license.skipAddThirdParty"=true --no-snapshot-updates
Review Comment:
The skip of spotless is not added, the following are the same situation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] FlechazoW commented on a diff in pull request #3939: [Improve][Code-Style] Add the spotless plugin for unified code style.
Posted by "FlechazoW (via GitHub)" <gi...@apache.org>.
FlechazoW commented on code in PR #3939:
URL: https://github.com/apache/incubator-seatunnel/pull/3939#discussion_r1099513942
##########
pom.xml:
##########
@@ -871,6 +820,77 @@
<groupId>org.codehaus.mojo</groupId>
<artifactId>flatten-maven-plugin</artifactId>
</plugin>
+
+ <plugin>
+ <groupId>com.diffplug.spotless</groupId>
+ <artifactId>spotless-maven-plugin</artifactId>
+ <version>${spotless.version}</version>
+ <configuration>
+ <java>
+ <googleJavaFormat>
+ <version>1.7</version>
+ <style>AOSP</style>
+ </googleJavaFormat>
+ <removeUnusedImports />
+ <formatAnnotations />
+ <importOrder>
+ <order>org.apache.seatunnel.shade,org.apache.seatunnel,org.apache,org,,javax,java,\#</order>
+ </importOrder>
+ <replaceRegex>
+ <name>Remove wildcard imports</name>
+ <searchRegex>import\s+(static)*\s*[^\*\s]+\*;(\r\n|\r|\n)</searchRegex>
+ <replacement>$1</replacement>
+ </replaceRegex>
+ <replaceRegex>
+ <name>Block powermock</name>
+ <searchRegex>import\s+org\.powermock\.[^\*\s]*(|\*);(\r\n|\r|\n)</searchRegex>
+ <replacement>$1</replacement>
+ </replaceRegex>
+ <replaceRegex>
+ <name>Block jUnit4 imports</name>
+ <searchRegex>import\s+org\.junit\.[^jupiter][^\*\s]*(|\*);(\r\n|\r|\n)</searchRegex>
+ <replacement>$1</replacement>
+ </replaceRegex>
+ </java>
+ <pom>
+ <sortPom>
+ <encoding>UTF-8</encoding>
+ <nrOfIndentSpace>4</nrOfIndentSpace>
+ <keepBlankLines>true</keepBlankLines>
+ <indentBlankLines>false</indentBlankLines>
+ <indentSchemaLocation>true</indentSchemaLocation>
+ <spaceBeforeCloseEmptyElement>true</spaceBeforeCloseEmptyElement>
+ <sortModules>false</sortModules>
+ <sortExecutions>false</sortExecutions>
+ <predefinedSortOrder>custom_1</predefinedSortOrder>
+ <expandEmptyElements>false</expandEmptyElements>
+ <sortProperties>false</sortProperties>
+ </sortPom>
+ <replace>
+ <name>Leading blank line</name>
+ <search>project</search>
+ <replacement>project</replacement>
+ </replace>
+ </pom>
+ <markdown>
+ <includes>
+ <include>docs/**/*.md</include>
+ </includes>
+ <excludes>
+ <exclude>**/.github/**/*.md</exclude>
+ </excludes>
+ <flexmark />
+ </markdown>
Review Comment:
Ok, I will add this configuration.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] FlechazoW commented on a diff in pull request #3939: [Improve][Code-Style] Add the spotless plugin for unified code style.
Posted by "FlechazoW (via GitHub)" <gi...@apache.org>.
FlechazoW commented on code in PR #3939:
URL: https://github.com/apache/incubator-seatunnel/pull/3939#discussion_r1090342683
##########
.github/workflows/backend.yml:
##########
@@ -302,14 +301,14 @@ jobs:
- name: run all modules unit test
if: needs.changes.outputs.api == 'true'
run: |
- ./mvnw -B -T 1C clean verify -D"maven.test.skip"=false -D"checkstyle.skip"=true -D"license.skipAddThirdParty"=true --no-snapshot-updates
+ ./mvnw -B -T 1C clean verify -D"maven.test.skip"=false -D"license.skipAddThirdParty"=true --no-snapshot-updates
Review Comment:
If 'mvn spotless:apply' has been executed before submitting the code, then the format of the code is in compliance with the specification, and there is no need to skip 'spotless:check'
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ruanwenjun commented on a diff in pull request #3939: [Improve][Code-Style] Add the spotless plugin for unified code style.
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on code in PR #3939:
URL: https://github.com/apache/incubator-seatunnel/pull/3939#discussion_r1098636012
##########
tools/spotless_check/pre-commit.sh:
##########
@@ -0,0 +1,24 @@
+#!/bin/sh
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# A hook script to automatically fix code style and formatting errors with spotless
+if ./mvnw spotless:check; then
+ exit 0
+fi
+./mvnw spotless:apply
+exit 1
Review Comment:
When I change the code and my format is not correct.
Then I execute below command
```
git add ./
git commit
```
This hook will check the format and use `spotless:apply` to format the project.
And the commit will failed, I need to execute below command again, the spotless will execute again.
```java
git add./
git commit
```
Can we remove `exit 1`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] FlechazoW commented on a diff in pull request #3939: [Improve][Code-Style] Add the spotless plugin for unified code style.
Posted by "FlechazoW (via GitHub)" <gi...@apache.org>.
FlechazoW commented on code in PR #3939:
URL: https://github.com/apache/incubator-seatunnel/pull/3939#discussion_r1090304869
##########
.github/workflows/backend.yml:
##########
@@ -302,14 +301,14 @@ jobs:
- name: run all modules unit test
if: needs.changes.outputs.api == 'true'
run: |
- ./mvnw -B -T 1C clean verify -D"maven.test.skip"=false -D"checkstyle.skip"=true -D"license.skipAddThirdParty"=true --no-snapshot-updates
+ ./mvnw -B -T 1C clean verify -D"maven.test.skip"=false -D"license.skipAddThirdParty"=true --no-snapshot-updates
Review Comment:
I have removed the plugin of 'checkstyle', it will throw an error of 'can not find plugin of checkstyle' if we still keep the 'checkstyle.skip'
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ruanwenjun commented on a diff in pull request #3939: [Improve][Code-Style] Add the spotless plugin for unified code style.
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on code in PR #3939:
URL: https://github.com/apache/incubator-seatunnel/pull/3939#discussion_r1099543936
##########
tools/spotless_check/pre-commit.sh:
##########
@@ -0,0 +1,24 @@
+#!/bin/sh
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# A hook script to automatically fix code style and formatting errors with spotless
+if ./mvnw spotless:check; then
+ exit 0
+fi
+./mvnw spotless:apply
+exit 1
Review Comment:
Ignore this comment, since we use spotless:apply, the file has been changed, so we need to add these again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] FlechazoW commented on pull request #3939: [Improve][Code-Style] Add the spotless plugin for unified code style.
Posted by "FlechazoW (via GitHub)" <gi...@apache.org>.
FlechazoW commented on PR #3939:
URL: https://github.com/apache/incubator-seatunnel/pull/3939#issuecomment-1407279060
@EricGao888 @TyrantLucifer PTAL. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] FlechazoW commented on pull request #3939: [Improve][Code-Style] Add the spotless plugin for unified code style.
Posted by "FlechazoW (via GitHub)" <gi...@apache.org>.
FlechazoW commented on PR #3939:
URL: https://github.com/apache/incubator-seatunnel/pull/3939#issuecomment-1408464361
PTAL. @EricJoy2048 @CalvinKirs @EricGao888
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer merged pull request #3939: [Improve][Code-Style] Add the spotless plugin for unified code style.
Posted by "TyrantLucifer (via GitHub)" <gi...@apache.org>.
TyrantLucifer merged PR #3939:
URL: https://github.com/apache/incubator-seatunnel/pull/3939
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] FlechazoW commented on pull request #3939: [Improve][Code-Style] Add the spotless plugin for unified code style.
Posted by "FlechazoW (via GitHub)" <gi...@apache.org>.
FlechazoW commented on PR #3939:
URL: https://github.com/apache/incubator-seatunnel/pull/3939#issuecomment-1408188053
> Are spotless and checkstyle complementary? Has this been discussed?
This has been discussed, see #3942 . Spotless and checkstyle are two different code styles. In order to maintain a unified style, we decided to use spotless and remove checkstyle.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ruanwenjun commented on a diff in pull request #3939: [Improve][Code-Style] Add the spotless plugin for unified code style.
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on code in PR #3939:
URL: https://github.com/apache/incubator-seatunnel/pull/3939#discussion_r1098626414
##########
pom.xml:
##########
@@ -871,6 +820,77 @@
<groupId>org.codehaus.mojo</groupId>
<artifactId>flatten-maven-plugin</artifactId>
</plugin>
+
+ <plugin>
+ <groupId>com.diffplug.spotless</groupId>
+ <artifactId>spotless-maven-plugin</artifactId>
+ <version>${spotless.version}</version>
+ <configuration>
+ <java>
+ <googleJavaFormat>
+ <version>1.7</version>
+ <style>AOSP</style>
+ </googleJavaFormat>
+ <removeUnusedImports />
+ <formatAnnotations />
+ <importOrder>
+ <order>org.apache.seatunnel.shade,org.apache.seatunnel,org.apache,org,,javax,java,\#</order>
+ </importOrder>
+ <replaceRegex>
+ <name>Remove wildcard imports</name>
+ <searchRegex>import\s+(static)*\s*[^\*\s]+\*;(\r\n|\r|\n)</searchRegex>
+ <replacement>$1</replacement>
+ </replaceRegex>
+ <replaceRegex>
+ <name>Block powermock</name>
+ <searchRegex>import\s+org\.powermock\.[^\*\s]*(|\*);(\r\n|\r|\n)</searchRegex>
+ <replacement>$1</replacement>
+ </replaceRegex>
+ <replaceRegex>
+ <name>Block jUnit4 imports</name>
+ <searchRegex>import\s+org\.junit\.[^jupiter][^\*\s]*(|\*);(\r\n|\r|\n)</searchRegex>
+ <replacement>$1</replacement>
+ </replaceRegex>
+ </java>
+ <pom>
+ <sortPom>
+ <encoding>UTF-8</encoding>
+ <nrOfIndentSpace>4</nrOfIndentSpace>
+ <keepBlankLines>true</keepBlankLines>
+ <indentBlankLines>false</indentBlankLines>
+ <indentSchemaLocation>true</indentSchemaLocation>
+ <spaceBeforeCloseEmptyElement>true</spaceBeforeCloseEmptyElement>
+ <sortModules>false</sortModules>
+ <sortExecutions>false</sortExecutions>
+ <predefinedSortOrder>custom_1</predefinedSortOrder>
+ <expandEmptyElements>false</expandEmptyElements>
+ <sortProperties>false</sortProperties>
+ </sortPom>
+ <replace>
+ <name>Leading blank line</name>
+ <search>project</search>
+ <replacement>project</replacement>
+ </replace>
+ </pom>
+ <markdown>
+ <includes>
+ <include>docs/**/*.md</include>
+ </includes>
+ <excludes>
+ <exclude>**/.github/**/*.md</exclude>
+ </excludes>
+ <flexmark />
+ </markdown>
Review Comment:
You can add upToDateChecking to improve the performance.
```java
<upToDateChecking>
<enabled>true</enabled>
</upToDateChecking>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] hailin0 commented on pull request #3939: [Improve][Code-Style] Add the spotless plugin for unified code style.
Posted by "hailin0 (via GitHub)" <gi...@apache.org>.
hailin0 commented on PR #3939:
URL: https://github.com/apache/incubator-seatunnel/pull/3939#issuecomment-1425243743
LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ruanwenjun commented on pull request #3939: [Improve][Code-Style] Add the spotless plugin for unified code style.
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on PR #3939:
URL: https://github.com/apache/incubator-seatunnel/pull/3939#issuecomment-1423489610
LGTM, you need to skip the spotless check in ci action, if you don't want to format the whole project in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org