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

upgrade to checkstyle 8.22 #231

Closed
romani opened this issue Jun 23, 2019 · 12 comments · Fixed by #242
Closed

upgrade to checkstyle 8.22 #231

romani opened this issue Jun 23, 2019 · 12 comments · Fixed by #242
Assignees
Labels
Milestone

Comments

@romani
Copy link
Member

romani commented Jun 23, 2019

https://checkstyle.org/releasenotes.html#Release_8.22 , upgrade version at https://github.com/checkstyle/sonar-checkstyle/edit/master/pom.xml#L88

Items to update in plugin:
New:
new check: MissingJavadocPackageCheck .
new check: UnnecessarySemicolonInTryWithResources.
new check: UnnecessarySemicolonInEnumeration.
new check: OrderedProperties.

@romani
Copy link
Member Author

romani commented Jun 23, 2019

@muhlba91 , please help with upgrade.
please do not forget to collect some artifacts of testing, to share in PR, as prove that simple testing was done.
as we have only new Check, we need to test config in Sonar UI and execution.

@muhlba91 muhlba91 self-assigned this Sep 10, 2019
muhlba91 pushed a commit to muhlba91/sonar-checkstyle that referenced this issue Sep 10, 2019
romani pushed a commit that referenced this issue Sep 10, 2019
@romani romani added this to the 4.22 milestone Sep 10, 2019
@romani
Copy link
Member Author

romani commented Sep 10, 2019

Fix is merged

@romani
Copy link
Member Author

romani commented Sep 10, 2019

@muhlba91 , do you have permissions to make a release ?

@muhlba91
Copy link
Contributor

I never tried. Today I got a Maven error due to Java 10 on my system and will try tomorrow on a Java 9 machine.

@muhlba91
Copy link
Contributor

@romani did you ever run into this issue:

[INFO] [ERROR] Failed to execute goal org.codehaus.mojo:cobertura-maven-plugin:2.7:instrument (default) on project checkstyle-sonar-plugin: Execution default of goal org.codehaus.mojo:cobertura-maven-plugin:2.7:instrument failed: Plugin org.codehaus.mojo:cobertura-maven-plugin:2.7 or one of its dependencies could not be resolved: Could not find artifact com.sun:tools:jar:0 at specified path /root/jdk-9.0.4/../lib/tools.jar -> [Help 1]

I received it with JDK 10/11 and apparently people reported this issue starting with Java 10, not many had it with Java 9 though.

@rnveach
Copy link
Member

rnveach commented Sep 13, 2019

at specified path /root/jdk-9.0.4/../lib/tools.jar

I have as far back as jdk 7. I had the tools jar in my JDK but something was looking for the file in the wrong place. To get around this issue, I just copied the JAR to the new location it was looking for.

@romani
Copy link
Member Author

romani commented Sep 13, 2019

I did release steps till step 6 (including), please continue.

On my side:

$ java -version
java version "1.8.0_162"
Java(TM) SE Runtime Environment (build 1.8.0_162-b12)
Java HotSpot(TM) 64-Bit Server VM (build 25.162-b12, mixed mode)

$ find /opt/jvm/jdk1.8.0_162/ -name "tools.jar"
/opt/jvm/jdk1.8.0_162/lib/tools.jar

but it is time to migrate to modern java... lets create separate issue on this.

@rnveach
Copy link
Member

rnveach commented Sep 13, 2019

@romani

it is time to migrate to modern java

Are you talking about all checkstyle or just sonar-checkstyle only?

@romani
Copy link
Member Author

romani commented Sep 13, 2019

only for sonar and only as installed jdk, not a target jdk.
checkstyle already build-able for on jdk11, we just need to find good time to make jdk11 as minimal required jdk in runtime. With jdk updates I usually wait for maven plugins super parent pom to upgrade to be able to build/release with new jdk. Probably it is already time ...

@muhlba91
Copy link
Contributor

PR: SonarSource/sonar-update-center-properties#51
SonarSource: https://community.sonarsource.com/t/new-release-sonar-checkstyle-4-22/14283

@romani yes, I'd favor updating the JDK requirements if possible.

I'll try working on and finishing #238 on Monday - are there any important points from your side?
Next week I'm not available (business trip) but if time permits, I'll work on #221 (the coverage check failed and I need to figure out what changes caused this).

@romani
Copy link
Member Author

romani commented Sep 13, 2019

are there any important points from your side?

Nothing special.

the coverage check failed and I need to figure out what changes caused this

Please move item that case coverage reduction to separate PR/issue. Let's merge simple stuff quickly.

@muhlba91
Copy link
Contributor

4.22 is released.

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

Successfully merging a pull request may close this issue.

3 participants