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

Issue #4846: ignore annotation in enum constructor for RedundantModifier #4902

Merged
merged 1 commit into from Aug 10, 2017

Conversation

rnveach
Copy link
Member

@rnveach rnveach commented Aug 5, 2017

Issue #4846

Regression to come.

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CIs need to be green.

item to improve:

while (modifier != null && modifier.getType() == TokenTypes.ANNOTATION) {
modifier = modifier.getNextSibling();
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic is not easy to understand, why first modifier is not modifier :) , it might be better to make method getFirstModifierAst() in this class, one day it will be moved to common Util class.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@codecov-io
Copy link

codecov-io commented Aug 5, 2017

Codecov Report

Merging #4902 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #4902   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         287     287           
  Lines       15489   15493    +4     
  Branches     3518    3519    +1     
======================================
+ Hits        15489   15493    +4
Impacted Files Coverage Δ
...kstyle/checks/modifier/RedundantModifierCheck.java 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e360526...a28d815. Read the comment docs.

@rnveach
Copy link
Member Author

rnveach commented Aug 5, 2017

Regression: http://rveach.no-ip.org/checkstyle/regression/reports/97/

Showed no differences.

@romani
Copy link
Member

romani commented Aug 5, 2017

as CI pass - ok to merge.

@rnveach
Copy link
Member Author

rnveach commented Aug 10, 2017

@romani ping

@romani romani merged commit 91b02ec into checkstyle:master Aug 10, 2017
@romani
Copy link
Member

romani commented Aug 10, 2017

@rnveach , as you see my approval for merge, and condition is satisfied (all CIs are green) - you can merge PR yourself.

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

Successfully merging this pull request may close these issues.

None yet

3 participants