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

Remove `cache` field from TreeWalker in Checkstyle 8.0 #2883

Closed
MEZk opened this issue Feb 7, 2016 · 8 comments

Comments

Projects
None yet
4 participants
@MEZk
Copy link
Contributor

commented Feb 7, 2016

base on problems at #2882 (comment)

The field 'cache' and 'setCacheFile' from TreeWalker now are used by maven-checkstyle-plugin.
We need to remove 'cache' field and method 'setCacheFile' from TreeWalker in Checkstyle 8.0 since they were moved to Checker in accordance with #569.

maven plugin already moved to use cache on Checker - https://github.com/apache/maven-plugins/blob/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugins/checkstyle/exec/DefaultCheckstyleExecutor.java#L388
fix was done at 2016-10-20 - apache/maven-plugins@17c9ffe
last release is 2.17 from 2015-10-15 - https://maven.apache.org/plugins/maven-checkstyle-plugin/history.html

method is already empty, so we have to just wait for new release of checkstyle maven plugin.

MIGRATION NOTES:
if user use old maven checkstyle plugin v 2.XX, It might experience error like (travis link):

[ERROR] 
Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:2.17:check
 (default-cli) on project maven-project: Failed during checkstyle configuration: 
cannot initialize module TreeWalker - Property 'cacheFile' does not exist, 
please check the documentation -> [Help 1]

to resolve issue, upgrade maven plugin 3.0.0 or later. Example of fix - sevntu-checkstyle/checkstyle-samples@c4f11a7

@romani romani added the approved label Feb 7, 2016

@rnveach

This comment has been minimized.

Copy link
Member

commented Nov 2, 2016

@romani This looks like a Checkstyle 8 candidate.

@romani romani added the checkstyle8 label Nov 2, 2016

@subkrish

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2017

I'm on it.

@romani

This comment has been minimized.

Copy link
Member

commented Jun 19, 2017

@subkrish , our CI is failed due to removed property - it is not acceptable.
what we should todo:

  1. create PRs to all project that we use in our CI( Postgresql JDBC Driverm , ... see failures of CI in PR) to move cache to Checker - share PR to them in this issue.
  2. If "1)" is taking time to merge on affected projects ..... we need to keep setter method that is deprecated and do nothing at all (just to not fail with exception)
  3. ones we release and 1) is merged we will remove TreeWalker empty setter for property completely.

FYI: Checker is having property for cache - http://checkstyle.sourceforge.net/config.html#Checker cacheFile.

@rnveach

This comment has been minimized.

Copy link
Member

commented Aug 3, 2017

@romani Comment at #4471 (comment) says we can't make change until maven is updated, but wercker failure at #4471 (comment) says it was problem with Postgresql JDBC Driver configuration.

Why do we need to wait on maven when we can just make changes to the configuration to pass? maven still worked fine in our CI.
We don't support cache in TreeWalker anyways, so it doesn't make a difference if it is removed now.

@romani

This comment has been minimized.

Copy link
Member

commented Aug 5, 2017

we depend on maven plugin only due to the fact we use it regression testing(sh, groovy, .....).
If non of CI fails ... we need to recheck that we actually substitute version to use. If we do substitution and all CI pass - we good to do any breaking changes now.

@rnveach

This comment has been minimized.

Copy link
Member

commented Dec 5, 2017

I was wrong. It is still a maven-plugin issue. We can't continue until they upgrade to 2.18.
Their issue is at https://issues.apache.org/jira/browse/MCHECKSTYLE-332 .

@rnveach

This comment has been minimized.

Copy link
Member

commented Dec 25, 2017

We have a new issue for asking maven-checkstyle-plugin to make a release at https://issues.apache.org/jira/browse/MCHECKSTYLE-346 .
This issue can't be completed until either they release a new version or we completely break away from using their plugin.

See checkstyle/contribution#273 , #5385 , #5386 .

rnveach added a commit to rnveach/checkstyle that referenced this issue Mar 10, 2019

rnveach added a commit to rnveach/checkstyle that referenced this issue Mar 11, 2019

rnveach added a commit to rnveach/checkstyle that referenced this issue Mar 14, 2019

romani added a commit that referenced this issue Mar 16, 2019

@romani romani added this to the 8.19 milestone Mar 16, 2019

@romani

This comment has been minimized.

Copy link
Member

commented Mar 16, 2019

fix is merged.

@romani romani closed this Mar 16, 2019

romani added a commit to sevntu-checkstyle/checkstyle-samples that referenced this issue Mar 31, 2019

romani added a commit to checkstyle/contribution that referenced this issue Mar 31, 2019

Vantuz added a commit to Vantuz/checkstyle that referenced this issue Apr 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.