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

[ISSUE #438] Enable Github Actions for Checkstyle #439

Merged
merged 10 commits into from
Jul 16, 2021

Conversation

SteveYurongSu
Copy link
Member

Fixes ISSUE #438.

Motivation

Explain the content here.
Explain why you want to make the changes and what problem you're trying to solve.

I noticed that there are folks working on issue #428 (#431), translating or removing Chinese chars in code.
Manually checking for this kind of code style can be a pain.
But, we can make the process of code style check automated with less pain by introducing Checkstyle into CI.

Modifications

Describe the modifications you've done.

  1. add a config file for checkstyle
  2. introduce the checkstyle job into CI workflow
  3. remove a useless file: style/copyright/Apache.xml

image

@SteveYurongSu SteveYurongSu marked this pull request as draft July 15, 2021 06:20
@codecov-commenter
Copy link

codecov-commenter commented Jul 15, 2021

Codecov Report

Merging #439 (6966048) into develop (de478dc) will increase coverage by 5.55%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             develop    #439      +/-   ##
============================================
+ Coverage       2.93%   8.49%   +5.55%     
- Complexity        89     256     +167     
============================================
  Files            224     228       +4     
  Lines          10754   10783      +29     
  Branches         921     918       -3     
============================================
+ Hits             316     916     +600     
+ Misses         10392    9793     -599     
- Partials          46      74      +28     
Impacted Files Coverage Δ
.../apache/eventmesh/client/tcp/common/TcpClient.java 0.00% <0.00%> (ø)
...e/eventmesh/client/http/consumer/LiteConsumer.java 0.00% <0.00%> (ø)
...eventmesh/client/tcp/impl/SimplePubClientImpl.java 0.00% <0.00%> (ø)
...eventmesh/client/tcp/impl/SimpleSubClientImpl.java 0.00% <0.00%> (ø)
...entmesh/runtime/core/plugin/MQConsumerWrapper.java 0.00% <0.00%> (ø)
...entmesh/runtime/core/plugin/MQProducerWrapper.java 0.00% <0.00%> (ø)
...he/eventmesh/connector/rocketmq/utils/OMSUtil.java 31.81% <0.00%> (ø)
...core/protocol/http/consumer/EventMeshConsumer.java 0.00% <0.00%> (ø)
...core/protocol/http/producer/EventMeshProducer.java 0.00% <0.00%> (ø)
.../protocol/tcp/client/group/ClientGroupWrapper.java 0.00% <0.00%> (ø)
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de478dc...6966048. Read the comment docs.

@ruanwenjun
Copy link
Member

We can also use the checkstyle plugin in IDEA to help us check our code style.
Does the checkstyle file need to be consistent with the codestyle file? It seems that we don't have a clear code specification currently.

@SteveYurongSu SteveYurongSu force-pushed the checkstyle branch 2 times, most recently from 5e5d718 to 77c1195 Compare July 15, 2021 08:35
@SteveYurongSu
Copy link
Member Author

@ruanwenjun Thanks for your comments!

We can also use the checkstyle plugin in IDEA to help us check our code style.

Sure! This can make our codebase healthy and clean!

Does the checkstyle file need to be consistent with the codestyle file?

I only added the following checks in the Checkstyle configuration file:

  • invoking System.out.println in source code
  • @author copyright
  • Chinese characters
  • PackageName must match "^(org).apache(.[a-zA-Z][a-zA-Z0-9]*)+$"

It seems that we don't have a clear code specification currently.

When we have a clear code style, we can add more constraints.
Personally, I prefer the Google coding style. 😊

@ruanwenjun
Copy link
Member

@SteveYurongSu Great

@SteveYurongSu SteveYurongSu marked this pull request as ready for review July 15, 2021 13:42
@qqeasonchen
Copy link
Contributor

@SteveYurongSu Will it become easier failed each build?

@SteveYurongSu
Copy link
Member Author

@SteveYurongSu Will it become easier failed each build?

I only added the following checks in the Checkstyle configuration file:

  • invoking System.out.println in source code
  • @author copyright
  • Chinese characters
  • PackageName must match "^(org).apache(.[a-zA-Z][a-zA-Z0-9]*)+$"

If a checkstyle workflow fails, there must be a finding related to one of the above cases, which means the author must modify the code. 😆

@qqeasonchen
Copy link
Contributor

LGTM

@qqeasonchen qqeasonchen merged commit 8bcdc2e into apache:develop Jul 16, 2021
jjz921024 pushed a commit to jjz921024/incubator-eventmesh that referenced this pull request Jul 25, 2021
* enable github actions for checkstyle

* try to fix ci

* ...

* ...

* ...

* Enable Github Actions for Checkstyle

* accelerate

* -filter-mode=nofilter -fail-on-error

* remove -name
xwm1992 pushed a commit to xwm1992/EventMesh that referenced this pull request Dec 27, 2021
* enable github actions for checkstyle

* try to fix ci

* ...

* ...

* ...

* Enable Github Actions for Checkstyle

* accelerate

* -filter-mode=nofilter -fail-on-error

* remove -name
xwm1992 pushed a commit that referenced this pull request Aug 4, 2022
* enable github actions for checkstyle

* try to fix ci

* ...

* ...

* ...

* Enable Github Actions for Checkstyle

* accelerate

* -filter-mode=nofilter -fail-on-error

* remove -name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants