Actually we could keep both Spotless and Checkstyle as long as we make the requirements of those two compatible. We will use Spotless for fixing style errors automatically and use Checkstyle in CI for double-check.

Actually we could keep both Spotless and Checkstyle as long as we make the requirements of those two compatible. We will use Spotless for fixing style errors automatically and use Checkstyle in CI for double-check.

AFAIK. Apache ShardingSphere is using this method, FYI:

https://github.com/apache/shardingsphere/blob/19265081c966cb0a839267b060593b258b0883b4/pom.xml#L1001-L1045

https://github.com/apache/shardingsphere/tree/master/src/resources

Also here is a related instructions written in Chinese:

https://shardingsphere.apache.org/blog/cn/material/spotless/

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 40.26%. Comparing base (bf5f7a8) to head (a4553be).
Report is 1604 commits behind head on dev.

@@            Coverage Diff            @@
##                dev   #11272   +/-   ##
=========================================
  Coverage     40.26%   40.26%           
+ Complexity     4843     4842    -1     
=========================================
  Files           974      974           
  Lines         37323    37323           
  Branches       4142     4142           
=========================================
  Hits          15028    15028           
+ Misses        20749    20748    -1     
- Partials       1546     1547    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Can we set the import order? And please change the related doc.

I've tested locally and it seems it does check the import order and fix it automatically, although I haven't figured it out how it does it. Anyway I will double check it. Thx

I will update the related docs in the following commits of this PR.

Can we set the import order? And please change the related doc.

I've tested locally and it seems it does check the import order and fix it automatically, although I haven't figured it out how it does it. Anyway I will double check it. Thx

I think spotless by default sorts the imports in alphabetical order unless we want to sort in another order we don't need extra configuration maybe

Can we set the import order? And please change the related doc.

I've tested locally and it seems it does check the import order and fix it automatically, although I haven't figured it out how it does it. Anyway I will double check it. Thx

I think spotless by default sorts the imports in alphabetical order unless we want to sort in another order we don't need extra configuration maybe

Ok, in fact, we have our own order of imports, but it's ok for me if we change to alphabetical order.

For developer's convenience, I suggest setting Spotless executions configuration as follows:

<executions>
  <execution>
      <goals>
          <goal>apply</goal>
      </goals>
      <phase>compile</phase>
  </execution>
</executions>

In this way, Spotless will automatically fix the style / formatting issues when compiling the project. However, we need to keep Checkstyle so that CI could make sure the style is correct in case developers do not compile and test the code before pushing it to remote repo.

Another way is to use git pre-commit hook and remove Checkstyle. WDYT @kezhenxu94 @ruanwenjun

For developer's convenience, I suggest setting Spotless executions configuration as follows:

<executions>

  <execution>

      <goals>

          <goal>apply</goal>

      </goals>

      <phase>compile</phase>

  </execution>

</executions>

In this way, Spotless will automatically fix the style / formatting issues when compiling the project. However, we need to keep Checkstyle so that CI could make sure the style is correct in case developers do not compile and test the code before pushing it to remote repo.

Another way is to use git pre-commit hook and remove Checkstyle. WDYT @kezhenxu94 @ruanwenjun

I personally used pre-commit hook in my own project and I prefer to use this. Also even we have the hook we should still keep the style check in CI because that can be skipped in developers' local machine.

For developer's convenience, I suggest setting Spotless executions configuration as follows:

<executions>

  <execution>

      <goals>

          <goal>apply</goal>

      </goals>

      <phase>compile</phase>

  </execution>

</executions>

Also, developers might skip running maven compile if they develop in an IDE (mostly)

Can you please also update this ?

dolphinscheduler/.github/actions/sanity-check/action.yml

Lines 34 to 53 in 8ded072

	- name: Check License Header
	uses: apache/skywalking-eyes@30367d8286e324d5efc58de4c70c37ea3648306d
	- uses: ./.github/actions/reviewdog-setup
	with:
	reviewdog_version: v0.10.2
	- shell: bash
	run: ./mvnw -B -q checkstyle:checkstyle-aggregate
	- shell: bash
	env:
	REVIEWDOG_GITHUB_API_TOKEN: ${{ inputs.token }}
	run: |
	if [[ -n "${{ inputs.token }}" ]]; then
	reviewdog -f=checkstyle \
	-reporter="github-pr-review" \
	-filter-mode="added" \
	-fail-on-error="true" < target/checkstyle-result.xml
	fi

Sure, just for confirmation, I'm supposed to update this with Spotless related stuff instead of simply removing it, correct?

Can you please also update this ?

dolphinscheduler/.github/actions/sanity-check/action.yml

Lines 34 to 53 in 8ded072

	- name: Check License Header
	uses: apache/skywalking-eyes@30367d8286e324d5efc58de4c70c37ea3648306d
	- uses: ./.github/actions/reviewdog-setup
	with:
	reviewdog_version: v0.10.2
	- shell: bash
	run: ./mvnw -B -q checkstyle:checkstyle-aggregate
	- shell: bash
	env:
	REVIEWDOG_GITHUB_API_TOKEN: ${{ inputs.token }}
	run: |
	if [[ -n "${{ inputs.token }}" ]]; then
	reviewdog -f=checkstyle \
	-reporter="github-pr-review" \
	-filter-mode="added" \
	-fail-on-error="true" < target/checkstyle-result.xml
	fi

Sure, just for confirmation, I'm supposed to update this with Spotless related stuff instead of simply removing it, correct?

Yes right

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication