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

[Improvement][all] Improvement checksyle #11211

Closed
3 tasks done
MichaelDeSteven opened this issue Jul 30, 2022 · 7 comments
Closed
3 tasks done

[Improvement][all] Improvement checksyle #11211

MichaelDeSteven opened this issue Jul 30, 2022 · 7 comments
Labels
question Further information is requested

Comments

@MichaelDeSteven
Copy link
Contributor

MichaelDeSteven commented Jul 30, 2022

Search before asking

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

Description

IMO, DS is not good enough in checksytle part.

image

When I want to compile or execute mvn command, I need to append - Dcheckstyle.skip. This is unfriendly for someone who is green hand and want to involved in this project.
I can improve this problem to make checkstyle pass in local environment.

Are you willing to submit a PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@MichaelDeSteven MichaelDeSteven added improvement make more easy to user or prompt friendly Waiting for reply Waiting for reply labels Jul 30, 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

@SbloodyS SbloodyS added question Further information is requested and removed improvement make more easy to user or prompt friendly Waiting for reply Waiting for reply labels Jul 30, 2022
@SbloodyS
Copy link
Member

The current checkstyle detection is incremental detection, and full detection is not required to avoid many PR code conflicts caused by large-scale modifications.

@MichaelDeSteven
Copy link
Contributor Author

MichaelDeSteven commented Jul 30, 2022

The current checkstyle detection is incremental detection, and full detection is not required to avoid many PR code conflicts caused by large-scale modifications.

Or we can just simply appending a note that To start server when you execute mvn command, you may need to append - Dcheckstyle.skip in developer guide docs?

@fuchanghai
Copy link
Member

@MichaelDeSteven @SbloodyS I suggest that when we modify a class, we do a code format check by the way。do we need to make a list like #10257 ?

@SbloodyS
Copy link
Member

SbloodyS commented Aug 1, 2022

The current checkstyle detection is incremental detection, and full detection is not required to avoid many PR code conflicts caused by large-scale modifications.

Or we can just simply appending a note that To start server when you execute mvn command, you may need to append - Dcheckstyle.skip in developer guide docs?

That sounds good to me. cc @EricGao888 @zhongjiajie

@SbloodyS
Copy link
Member

SbloodyS commented Aug 1, 2022

@MichaelDeSteven @SbloodyS I suggest that when we modify a class, we do a code format check by the way。do we need to make a list like #10257 ?

Currently, our CI detection is incremental.

@EricGao888
Copy link
Member

The current checkstyle detection is incremental detection, and full detection is not required to avoid many PR code conflicts caused by large-scale modifications.

Or we can just simply appending a note that To start server when you execute mvn command, you may need to append - Dcheckstyle.skip in developer guide docs?

That sounds good to me. cc @EricGao888 @zhongjiajie

+1, before we fix all the style errors as planned in #10573 , I think instructing developers to use -Dcheckstyle.skip is a good idea. Thx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants