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.25 #244

Closed
romani opened this issue Sep 29, 2019 · 10 comments · Fixed by #247
Closed

upgrade to checkstyle 8.25 #244

romani opened this issue Sep 29, 2019 · 10 comments · Fixed by #247
Labels
Milestone

Comments

@romani
Copy link
Member

romani commented Sep 29, 2019

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

@romani
Copy link
Member Author

romani commented Sep 29, 2019

Plugin is affected by:

Breaking backward compatibility:
JavadocMethodCheck: remove deprecated properties ignoreMethodNamesRegex, minLineCount, allowMissingJavadoc, allowMissingPropertyJavadoc.
New:
RegexpMultiline not detecting matches across multiple lines.

@muhlba91
Copy link
Contributor

upgrade is not affected by JavadocMethodCheck since those properties have been removed earlier and are not present anymore.

@muhlba91
Copy link
Contributor

After upgrade, I get the following error on running the tests and also using the sonarqube scanner (gradle version):
com.puppycrawl.tools.checkstyle.XMLLogger.<init>(Ljava/io/OutputStream;Z)V

@rnveach
Copy link
Member

rnveach commented Oct 28, 2019

Yes, it was a breaking compatibility in the release. I am not sure why it is not mentioned above.

https://checkstyle.org/releasenotes.html#Release_8.25

DefaultLogger: remove deprecated constructors. Author: rnveach
ConfigurationLoader: remove deprecated constructors. Author: rnveach
XMLLogger: remove deprecated constructor. Author: rnveach
Remove "update" methods from DetailAST as developers shouldn't be modifying the tree from inside a check. Author: rnveach
FileContents: remove deprecated constructor and deprecated methods. Author: rnveach
JavadocMethodCheck: remove deprecated properties ignoreMethodNamesRegex, minLineCount, allowMissingJavadoc, allowMissingPropertyJavadoc. Author: rnveach
Remove AbstractTypeParameterNameCheck since it is Deprecated. Author: rnveach

@rnveach
Copy link
Member

rnveach commented Oct 28, 2019

The following code

checker.addListener(new XMLLogger(xmlOutput, true));

will have to be changed into
checkstyle/checkstyle@b8ac94c#diff-b38bc5ff474af8ee12faa35c1144acfcL1351-R1356

@muhlba91
Copy link
Contributor

thank you, now it's working :)

muhlba91 pushed a commit to muhlba91/sonar-checkstyle that referenced this issue Oct 28, 2019
romani pushed a commit that referenced this issue Oct 28, 2019
@romani romani added this to the 4.25 milestone Oct 28, 2019
@romani
Copy link
Member Author

romani commented Oct 29, 2019

@muhlba91 , I did release 4.25 as always till point 6 (including). Please proceed.
I would be awesome if you finally start running of maven release yourself, so it will speedup the process.

Unfortunately amount of hanging PRs is keep growing in main project and it my review of them blocking merges. I cannot keep up to support all checkstyle environment projects. I even have to unblock development of eclipse-cs plugin, as we kind of lost all maintainers. I have to switch my focus to it. Do not worry, I do not plan to drop sonar plugin, I will be always around, I just do not want plugin be blocked by me.

@muhlba91
Copy link
Contributor

muhlba91 commented Oct 29, 2019

PR: SonarSource/sonar-update-center-properties#64
SS: https://community.sonarsource.com/t/new-release-sonar-checkstyle-4-25/16018

@romani I understand the problem of having to focus on too many areas and it's good to hear you'll be around. :) i really appreciate your work and effort for this plugin.
i assume #248 is related to this topic now? will you still create the checkstyle upgrade issues here or will this have to be taken over?

as for the releases. what's needed to fix my release problem mentioned here: #231 (comment)
as a quick fix, should it work to simply copy the library to the "wrong" path or will this create more problems?

@muhlba91
Copy link
Contributor

version is released. :)

@romani
Copy link
Member Author

romani commented Oct 30, 2019

will you still create the checkstyle upgrade issues here or will this have to be taken over?

I will create issue, I do it as release routine.

as a quick fix, should it work to simply copy the library to the "wrong" path or will this create more problems?

Yes, you can put such jar in required by plugin path just to make it happy, it weird why it need it, cobertura/cobertura#403 , but cobertura project is so outdated, ... almost dead.

Not simple but good solution is to do #109 , so will forget about such problem completely.

As we make it happen, we can upgrade Travis to use openjdk11,12,... as runtime, so build problem will never be surprise again.

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