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.

Sign up for GitHub

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

Jump to bottom

use checkstyle in maven to check code #802

Merged
merged 5 commits into from
Apr 24, 2019
Merged

use checkstyle in maven to check code #802

merged 5 commits into from
Apr 24, 2019

Conversation

leizhiyuan
Copy link
Contributor

@leizhiyuan leizhiyuan commented Apr 16, 2019 •
edited
Loading

Ⅰ. Describe what this PR did

  1. use checkstyle in maven to check code
  2. fix copyright check error
  3. remove @ author check

Ⅱ. Does this pull request fix one issue?

fixes #797

Ⅲ. Why don't you add test cases (unit test/integration test)?

do not need

Ⅳ. Describe how to verify it

mvn clean install -DskipTests=true you will see the result. some code needs to be cleaned next pr

Ⅴ. Special notes for reviews

@codecov-io
Copy link

codecov-io commented Apr 16, 2019 •
edited
Loading

Codecov Report

Merging #802 into develop will increase coverage by 0.17%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #802      +/-   ##
=============================================
+ Coverage      38.65%   38.82%   +0.17%     
- Complexity      1026     1028       +2     
=============================================
  Files            220      220              
  Lines           8507     8507              
  Branches        1028     1028              
=============================================
+ Hits            3288     3303      +15     
+ Misses          4828     4813      -15     
  Partials         391      391
Impacted Files Coverage Δ Complexity Δ
...eata/core/rpc/netty/AbstractRpcRemotingClient.java 20.57% <0%> (+3.34%) 7% <0%> (ø) ⬇️
...ain/java/io/seata/core/rpc/netty/ShutdownHook.java 81.25% <0%> (+25%) 8% <0%> (+2%) ⬆️

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 9f7ee01...d592df0. Read the comment docs.

@zhangthen zhangthen added this to the 0.6.0 milestone Apr 16, 2019
@zhangthen
Copy link
Contributor

Well done, we will merge it between 0.5.0 and 0.6.0.

@leizhiyuan
use checkstyle in maven to check code
4b38cbd 
fix copyright check error
remove @author check
@leizhiyuan
Copy link
Contributor Author

license need to be formatted in other commit

@zhangthen
Copy link
Contributor

Which codestyle do you use ?

@leizhiyuan
Copy link
Contributor Author

leizhiyuan commented Apr 17, 2019 •
edited
Loading

Which codestyle do you use ?

https://github.com/alibaba/p3c/blob/master/p3c-formatter/eclipse-codestyle.xml this style

p3c seems has no other codestyle for maven checkstyle plugin, this file is not style correctly. I revert it

@leizhiyuan
Copy link
Contributor Author

p3c has not support for checkstyle alibaba/p3c#265

in SOFARPC, we use

  <plugin>
                <groupId>com.googlecode.maven-java-formatter-plugin</groupId>
                <artifactId>maven-java-formatter-plugin</artifactId>
                <version>0.4</version>
                <executions>
                    <execution>
                        <goals>
                            <goal>format</goal>
                        </goals>
                    </execution>
                </executions>
                <configuration>
                    <configFile>${user.dir}/tools/codestyle/formatter.xml</configFile>
                    <encoding>UTF-8</encoding>
                </configuration>
            </plugin>

this will format when maven clean install, do you have any idea?

zhangthen
zhangthen approved these changes Apr 23, 2019
View reviewed changes
Copy link
Contributor

@zhangthen zhangthen left a comment

Choose a reason for hiding this comment

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

Well done.

@ujjboy ujjboy requested a review from wgs13579 April 24, 2019 06:12
wgs13579
wgs13579 approved these changes Apr 24, 2019
View reviewed changes
Copy link
Contributor

@wgs13579 wgs13579 left a comment

Choose a reason for hiding this comment

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

LGTM

@zhangthen zhangthen merged commit 8efcda1 into apache:develop Apr 24, 2019
@fescar-robot fescar-robot mentioned this pull request Apr 26, 2019
Closed
@leizhiyuan leizhiyuan mentioned this pull request May 7, 2019
Closed
nick-tan pushed a commit to nick-tan/seata that referenced this pull request Jul 12, 2019
@leizhiyuan @nick-tan
use checkstyle in maven to check code (apache#802)
f1f3a72
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhangthen zhangthen zhangthen approved these changes

@wgs13579 wgs13579 wgs13579 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.6.*
Development

Successfully merging this pull request may close these issues.

Use CheckStyle to style the code
4 participants
@leizhiyuan @codecov-io @zhangthen @wgs13579