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

api changes: make getAcceptableTokens/getRequiredTokens/getDefaultTokens methods as abstract in Check.java #605

Closed
romani opened this Issue Jan 29, 2015 · 8 comments

Comments

Projects
None yet
5 participants

@romani romani added the approved label Jan 29, 2015

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Aug 6, 2016

Contributor

@romani
Is is still actual? We have already passes 7.0 release. Maybe we should make getAcceptableTokens and getRequiredTokens as abstract?

See your commit eec72ee

Contributor

MEZk commented Aug 6, 2016

@romani
Is is still actual? We have already passes 7.0 release. Maybe we should make getAcceptableTokens and getRequiredTokens as abstract?

See your commit eec72ee

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Aug 7, 2016

Member

It should be done in 8.0 version , where we will brake backward compatibility by api. 7.x version is only for jdk8 update

Member

romani commented Aug 7, 2016

It should be done in 8.0 version , where we will brake backward compatibility by api. 7.x version is only for jdk8 update

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Nov 1, 2016

Member

Methods in question reference getAcceptableTokens and getRequiredTokens.
Javadoc token methods in AbstractJavadocCheck should also mirror any changes in Check token methods.
Acceptable >= Default >= Required

Member

rnveach commented Nov 1, 2016

Methods in question reference getAcceptableTokens and getRequiredTokens.
Javadoc token methods in AbstractJavadocCheck should also mirror any changes in Check token methods.
Acceptable >= Default >= Required

@romani romani added the checkstyle8 label Nov 1, 2016

@subkrish

This comment has been minimized.

Show comment
Hide comment
@subkrish

subkrish Jun 24, 2017

Contributor

@romani @rnveach I think this issue should be closed as this file is no longer used and has been deprecated.

Contributor

subkrish commented Jun 24, 2017

@romani @rnveach I think this issue should be closed as this file is no longer used and has been deprecated.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 24, 2017

Member

@subkrish , we need to remove Check.java completely from repo.
in issue description is stated that we need to make getXXXXXTokens as abstract, to force all thirdparty Checks to implement them (all standard checks already implement such methods).

Member

romani commented Jun 24, 2017

@subkrish , we need to remove Check.java completely from repo.
in issue description is stated that we need to make getXXXXXTokens as abstract, to force all thirdparty Checks to implement them (all standard checks already implement such methods).

rnveach added a commit to rnveach/checkstyle that referenced this issue Jul 1, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Jul 1, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Jul 1, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Jul 1, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Jul 1, 2017

romani added a commit that referenced this issue Jul 1, 2017

@romani romani added this to the 8.0 milestone Jul 2, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 2, 2017

Member

Fix is merged

Member

romani commented Jul 2, 2017

Fix is merged

@romani romani closed this Jul 2, 2017

@romani romani changed the title from api changes: make methods as abstract in Check.java to api changes: make getAcceptableTokens/getRequiredTokens/getDefaultTokens methods as abstract in Check.java Jul 2, 2017

@liutikas

This comment has been minimized.

Show comment
Hide comment
@liutikas

liutikas Aug 11, 2017

Can you also please update http://checkstyle.sourceforge.net/writingchecks.html as now the example Check does not even compile?

Can you also please update http://checkstyle.sourceforge.net/writingchecks.html as now the example Check does not even compile?

@romani

This comment has been minimized.

Show comment
Hide comment
Member

romani commented Aug 12, 2017

gerrit-ovirt-org pushed a commit to oVirt/ovirt-engine that referenced this issue Apr 17, 2018

build: future proof against Checktyle 8 API change
Checkstyle 8 will break the public API of AbstractCheck and require
implementing  getAcceptableTokens(), getDefaultTokens() and
getRequiredTokens() so that Acceptable >= Default >= Required (see [1]
for reference).

This patch future proofs oVirt's checks and implements all three in a
naive way so that upgrading to Checkstyle 8 won't break their
compilation.

[1] checkstyle/checkstyle#605

Change-Id: I594095aee1fafd535b336c974859f91d4cf223c4
Signed-off-by: Allon Mureinik <amureini@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment