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 deprecated class BaseCheckTestSupport #4867

Closed
subkrish opened this Issue Jul 31, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@subkrish
Contributor

subkrish commented Jul 31, 2017

https://github.com/checkstyle/checkstyle/blob/master/src/test/java/com/puppycrawl/tools/checkstyle/BaseCheckTestSupport.java

BaseCheckTestSupport is now deprecated and longer supported by any of the test classes.

It must be removed once xwiki-commons uses the new Abstract...TestSupport classes, else wercker build will fail as it did in an attempt to remove the class in PR #4655
https://github.com/checkstyle/checkstyle/pull/4655/files#diff-b05f7d983d1f3ec19e57707a34da40b2

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.6.1:testCompile (default-testCompile) on project xwiki-commons-tool-verification-resources: Compilation failure: Compilation failure:
[ERROR] /pipeline/source/xwiki-commons/xwiki-commons-tools/xwiki-commons-tool-verification-resources/src/test/java/org/xwiki/tool/checkstyle/SinceFormatCheckTest.java:[28,39] cannot find symbol
[ERROR] symbol:   class BaseCheckTestSupport
[ERROR] location: package com.puppycrawl.tools.checkstyle
@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jul 31, 2017

Member

must be removed once xwiki-commons uses the new Abstract...TestSupport classes

xwiki will most likely not update unless we either break their build or send them a PR to force them to change.
@romani Should we send them a PR to force them to change?

Member

rnveach commented Jul 31, 2017

must be removed once xwiki-commons uses the new Abstract...TestSupport classes

xwiki will most likely not update unless we either break their build or send them a PR to force them to change.
@romani Should we send them a PR to force them to change?

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Aug 2, 2017

Member

Should we send them a PR to force them to change?

It would be better to do this, as we know how to do a change. For them it took a while to understand what we did.
@subkrish , can you send PR to xwiki project ?
looks like we released new TestSupport classes already so they could switch.

Member

romani commented Aug 2, 2017

Should we send them a PR to force them to change?

It would be better to do this, as we know how to do a change. For them it took a while to understand what we did.
@subkrish , can you send PR to xwiki project ?
looks like we released new TestSupport classes already so they could switch.

@subkrish

This comment has been minimized.

Show comment
Hide comment
@subkrish

subkrish Aug 2, 2017

Contributor

@romani @rnveach I just made the PR -> xwiki/xwiki-commons#19 making the necessary changes by updating the Checkstyle version and by removing the test to extend BaseCheckTestSupport and to now extend AbstractModuleTestSupport.

Contributor

subkrish commented Aug 2, 2017

@romani @rnveach I just made the PR -> xwiki/xwiki-commons#19 making the necessary changes by updating the Checkstyle version and by removing the test to extend BaseCheckTestSupport and to now extend AbstractModuleTestSupport.

@subkrish

This comment has been minimized.

Show comment
Hide comment
@subkrish

subkrish Aug 3, 2017

Contributor

@romani @rnveach xwiki/xwiki-commons#19 has been merged. I will now remove the BaseCheckTestSupport class as I am pretty sure no other project uses it anymore.

Contributor

subkrish commented Aug 3, 2017

@romani @rnveach xwiki/xwiki-commons#19 has been merged. I will now remove the BaseCheckTestSupport class as I am pretty sure no other project uses it anymore.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Aug 3, 2017

Member

@subkrish , you can try, as CIs are green we good to merge.

Member

romani commented Aug 3, 2017

@subkrish , you can try, as CIs are green we good to merge.

@rnveach rnveach added the approved label Aug 3, 2017

subkrish added a commit to subkrish/checkstyle that referenced this issue Aug 3, 2017

subkrish added a commit to subkrish/checkstyle that referenced this issue Aug 3, 2017

subkrish added a commit to subkrish/checkstyle that referenced this issue Oct 1, 2017

subkrish added a commit to subkrish/checkstyle that referenced this issue Oct 2, 2017

romani added a commit that referenced this issue Oct 9, 2017

@romani romani added the miscellaneous label Oct 9, 2017

@romani romani added this to the 8.4 milestone Oct 9, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Oct 9, 2017

Member

fix is merged

Member

romani commented Oct 9, 2017

fix is merged

@romani romani closed this Oct 9, 2017

timurt added a commit to timurt/checkstyle that referenced this issue Dec 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment