Skip to content

Dogfood pmd plugin #466

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Dogfood pmd plugin #466

wants to merge 2 commits into from

Conversation

blindpirate
Copy link
Contributor

Context

It's not convincing if p3c project doesn't dogfood its own pmd plugin. This PR applies pmd plugin and rules to p3c-pmd project and fixes two violations.

@CLAassistant
Copy link

CLAassistant commented Mar 13, 2019

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@blindpirate
Copy link
Contributor Author

I can also set up a CI job (Travis/CircleCI/Appveyor, etc.) if necessary.

This commit applies pmd plugin and rules to p3c-pmd project and fixes two violations.
p3c-pmd/pom.xml Outdated
<version>3.8</version>
<configuration>
<sourceEncoding>${project.build.sourceEncoding}</sourceEncoding>
<targetJdk>1.8</targetJdk>
Copy link
Contributor Author

@blindpirate blindpirate Mar 13, 2019

Choose a reason for hiding this comment

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

Not sure if this is good, please advise.

Copy link
Collaborator

Choose a reason for hiding this comment

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

你好,目前我们的插件是从JDK1.7版本开始支持。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的。根据https://maven.apache.org/plugins/maven-pmd-plugin/examples/targetJdk.html 的建议,将这个值改为:${maven.compiler.target}

<version>3.8</version>
<configuration>
<sourceEncoding>${project.build.sourceEncoding}</sourceEncoding>
<targetJdk>${maven.compiler.target}</targetJdk>
Copy link
Contributor

@kerie kerie Apr 2, 2019

Choose a reason for hiding this comment

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

还需要在<properties>里加上maven.compiler.target这个属性吧?

<maven.compiler.source>1.7</maven.compiler.source>
<maven.compiler.target>1.7</maven.compiler.target>

下面maven-compiler-plugin里也统一改成这种形式。

@SeanCai
Copy link
Contributor

SeanCai commented Jun 17, 2019

@blindpirate

你好,插件最新版已经更新
pmd-> 6.15.0
jdk 最低支持 1.8
另外 p3c-pmd 加入了 p3c规约检测

你那边帮忙看看是否有需要改进的,因为改动较大,所以你本次pr可能不再适用了,十分抱歉

@XenoAmess
Copy link

XenoAmess commented Apr 10, 2020

suggest do NEVER do clinic dependency with a specific version.
use it with a range instead.

                    <dependency>
                        <groupId>com.alibaba.p3c</groupId>
                        <artifactId>p3c-pmd</artifactId>
                        <version>[1,)</version>
                    </dependency>

or even:

                    <dependency>
                        <groupId>${project.groupId}</groupId>
                        <artifactId>${project.artifactId}</artifactId>
                        <version>[1,${project.version}]</version>
                    </dependency>

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.

6 participants