Skip to content
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

[Feature][style] Replace checkstyle plugin with spotless to automatically fix formatting errors #10963

Closed
3 tasks done
EricGao888 opened this issue Jul 14, 2022 · 5 comments · Fixed by #11272
Closed
3 tasks done
Assignees
Labels
discussion discussion document feature new feature
Milestone

Comments

@EricGao888
Copy link
Member

EricGao888 commented Jul 14, 2022

Search before asking

  • I had searched in the issues and found no similar feature requirement.

Description

Use case

  • At present, DS uses checkstyle plugin. However, there are tons of legacy formatting errors in the project every time we build the project. Thus, in practice, some developers tend to use -Dcheckstyle.skip flag when building the project and this make the style check a little bit awkward.
  • Huge advantage of using Spotless over CheckStyle is in addition to check the formatting of the code it also has apply goal that fixes all the style and formatting. With Spotless, we could fix the legacy formatting errors once for all and developers could fix new format errors with a single line of command mvn spotless:apply.

Related issues

No response

Are you willing to submit a PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@EricGao888 EricGao888 added feature new feature Waiting for reply Waiting for reply labels Jul 14, 2022
@github-actions
Copy link

Thank you for your feedback, we have received your issue, Please wait patiently for a reply.

  • In order for us to understand your request as soon as possible, please provide detailed information、version or pictures.
  • If you haven't received a reply for a long time, you can join our slack and send your question to channel #troubleshooting

@EricGao888 EricGao888 self-assigned this Jul 14, 2022
@EricGao888 EricGao888 removed the Waiting for reply Waiting for reply label Jul 14, 2022
@EricGao888 EricGao888 added this to the 3.1.0-alpha milestone Jul 14, 2022
@EricGao888 EricGao888 changed the title [Feature][style] Add spotless plugin to automatically fix formatting errors [Feature][style] Replace checkstyle plugin with spotless to automatically fix formatting errors Jul 15, 2022
@EricGao888 EricGao888 added the discussion discussion label Jul 15, 2022
EricGao888 added a commit to EricGao888/dolphinscheduler that referenced this issue Jul 15, 2022
@zhuxt2015
Copy link
Contributor

I agree to replace CheckStyle with Spotless. Resolve format problem of the code as soon as possible

@EricGao888
Copy link
Member Author

EricGao888 commented Jul 23, 2022

BTW, we could fix the formatting and style errors incrementally, together with unit tests. See: #10573
One convenient way is when refactoring some UTs, we fix the formatting and style errors of both the testing and tested part. WDYT @ruanwenjun @zhongjiajie @zhuxt2015

@EricGao888
Copy link
Member Author

Some updates, see: #11272 (comment)

@zhongjiajie
Copy link
Member

zhongjiajie commented Aug 3, 2022

@ruanwenjun have other oppoin

EricGao888 added a commit to EricGao888/dolphinscheduler that referenced this issue Aug 5, 2022
EricGao888 added a commit to EricGao888/dolphinscheduler that referenced this issue Aug 5, 2022
EricGao888 added a commit to EricGao888/dolphinscheduler that referenced this issue Aug 5, 2022
EricGao888 added a commit to EricGao888/dolphinscheduler that referenced this issue Aug 5, 2022
EricGao888 added a commit that referenced this issue Aug 6, 2022
…11272)

* [Feature][style] Add spotless maven plugin for automatic style fix (#10963)

* Fix spotless ratchet configuration

* Remove license-check and decrease line length threshold value

* Update related docs

* Remove checkstyle and add pre-commit hook

* Test updated pre-commit hook

* Replace checkstyle with spotless in CI

* Remove reviewdog
ruanwenjun added a commit to ruanwenjun/dolphinscheduler that referenced this issue Sep 7, 2022
* [Feature][style] Add spotless maven plugin for automatic style fix. (apache#11272)

* [Feature][style] Add spotless maven plugin for automatic style fix (apache#10963)

* Fix spotless ratchet configuration

* Remove license-check and decrease line length threshold value

* Update related docs

* Remove checkstyle and add pre-commit hook

* Test updated pre-commit hook

* Replace checkstyle with spotless in CI

* Remove reviewdog

(cherry picked from commit 6a02870)

* Cp spotless

Co-authored-by: Eric Gao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion discussion document feature new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants