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 #442] Fix findings filtered by Checkstyle workflow #443

Merged
merged 18 commits into from
Jul 19, 2021

Conversation

SteveYurongSu
Copy link
Member

@SteveYurongSu SteveYurongSu commented Jul 17, 2021

Fixes ISSUE #442.

Motivation

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

I introduced CheckStyle workflow in PR #438, but did not fix the issues that Checkstyle found.

Modifications

Describe the modifications you've done.

  • Fixed all findings filtered by the Checkstyle workflow.

  • Modified Checkstyle config file:

<module name="RegexpSingleline">
    <property name="format" value="System\..+\.println"/>
    <property name="message" value="Prohibit invoking System.*.println in source code !"/>
</module>

@codecov-commenter
Copy link

Codecov Report

Merging #443 (9e42c41) into develop (ed446ba) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             develop    #443   +/-   ##
=========================================
  Coverage       9.80%   9.80%           
  Complexity       282     282           
=========================================
  Files            228     228           
  Lines          10785   10785           
  Branches         919     919           
=========================================
  Hits            1057    1057           
  Misses          9633    9633           
  Partials          95      95           
Impacted Files Coverage Δ
...esh/common/protocol/http/common/ClientRetCode.java 0.00% <0.00%> (ø)
...ntmesh/common/protocol/http/common/ClientType.java 0.00% <0.00%> (ø)
...tmesh/common/protocol/http/common/RequestCode.java 0.00% <0.00%> (ø)
...ime/core/consumergroup/ConsumerGroupTopicConf.java 0.00% <0.00%> (ø)
...ocol/http/processor/BatchSendMessageProcessor.java 0.00% <ø> (ø)
...time/core/protocol/tcp/client/session/Session.java 0.00% <ø> (ø)
...client/session/push/retry/EventMeshTcpRetryer.java 0.00% <ø> (ø)
...rotocol/tcp/client/session/send/SessionSender.java 0.00% <ø> (ø)
...eventmesh/runtime/metrics/http/SummaryMetrics.java 0.00% <ø> (ø)
...g/apache/eventmesh/client/http/RemotingServer.java 0.00% <ø> (ø)
... and 3 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 ed446ba...9e42c41. Read the comment docs.

@ruanwenjun
Copy link
Member

I also do the same thing at #433, if this pr is merged first, I will rebase.

@qqeasonchen
Copy link
Contributor

It seems this pr needs be merged first or every other new pr will be checked failed.

@ruanwenjun
Copy link
Member

@qqeasonchen Yes, If there are no other problems, this pr can be merged first, otherwise the other commits will be blocked.

@qqeasonchen
Copy link
Contributor

Have any other guys reviewed already?

Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, When I submitted a pr, I found that the file sChat2.jks seems to break the rules in checkStyle, do we need to exclude this file? I am not sure whether this will happen.

@SteveYurongSu
Copy link
Member Author

LGTM, When I submitted a pr, I found that the file sChat2.jks seems to break the rules in checkStyle, do we need to exclude this file? I am not sure whether this will happen.

@ruanwenjun Thanks for reviewing!

I have the same question. It seems that sChat2.jks is an unexpected binary file, should we remove the file? @qqeasonchen

@ruanwenjun
Copy link
Member

LGTM, When I submitted a pr, I found that the file sChat2.jks seems to break the rules in checkStyle, do we need to exclude this file? I am not sure whether this will happen.

@ruanwenjun Thanks for reviewing!

I have the same question. It seems that sChat2.jks is an unexpected binary file, should we remove the file? @qqeasonchen

If this file is not needed, we can remove it, and if this is needed then we can filter it like below:

<module name="BeforeExecutionExclusionFileFilter">
    <property name="fileNamePattern" value="./eventmesh-runtime/conf/sChat2.jks$"/>
  </module>

Or using checkStyle-suppressions.xml

@qqeasonchen
Copy link
Contributor

sChat2.jks can be removed.

@qqeasonchen qqeasonchen merged commit db42296 into apache:develop Jul 19, 2021
jjz921024 pushed a commit to jjz921024/incubator-eventmesh that referenced this pull request Jul 25, 2021
…he#443)

* fix findings in RequestCode

* fix findings in ClientRetCode

* fix findings in Async*

* fix findings in eventmesh-runtime

* translate all chinese

* translate

* fix findings in org.apache.eventmesh.spi.example

* move package demo to org.apache.eventmesh.runtime.demo

* translate

* rename package client to org.apache.eventmesh.runtime.client

* rename package client to org.apache.eventmesh.runtime.client

* update checkStyle.xml

* fix System.err.out

* delete sChat2

* remove comments in eventmesh-sdk-java:build.gradle

* remove Chinese in eventmesh-test:build.gradle

* fix Chinese findings in tcp_sub_broadcast.sh

* fix Chinese findings in *.sh
xwm1992 pushed a commit to xwm1992/EventMesh that referenced this pull request Dec 27, 2021
…he#443)

* fix findings in RequestCode

* fix findings in ClientRetCode

* fix findings in Async*

* fix findings in eventmesh-runtime

* translate all chinese

* translate

* fix findings in org.apache.eventmesh.spi.example

* move package demo to org.apache.eventmesh.runtime.demo

* translate

* rename package client to org.apache.eventmesh.runtime.client

* rename package client to org.apache.eventmesh.runtime.client

* update checkStyle.xml

* fix System.err.out

* delete sChat2

* remove comments in eventmesh-sdk-java:build.gradle

* remove Chinese in eventmesh-test:build.gradle

* fix Chinese findings in tcp_sub_broadcast.sh

* fix Chinese findings in *.sh
xwm1992 pushed a commit that referenced this pull request Aug 4, 2022
* fix findings in RequestCode

* fix findings in ClientRetCode

* fix findings in Async*

* fix findings in eventmesh-runtime

* translate all chinese

* translate

* fix findings in org.apache.eventmesh.spi.example

* move package demo to org.apache.eventmesh.runtime.demo

* translate

* rename package client to org.apache.eventmesh.runtime.client

* rename package client to org.apache.eventmesh.runtime.client

* update checkStyle.xml

* fix System.err.out

* delete sChat2

* remove comments in eventmesh-sdk-java:build.gradle

* remove Chinese in eventmesh-test:build.gradle

* fix Chinese findings in tcp_sub_broadcast.sh

* fix Chinese findings in *.sh
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