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

Rename Filters which aren't module filters to FilterElement #4978

Closed
rnveach opened this Issue Aug 21, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@rnveach
Copy link
Member

rnveach commented Aug 21, 2017

https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressElement.java#L42-L43
It is slightly confusing to see SuppressElement implement the filter interface even though it isn't a direct filter. The true filter is SuppressionFilter which fully meets the requirements of a filter by extending AutomaticBean.
SuppressElement is more a sub-child of SuppressionFilter and therefore is more a FilterElement.

The following classes should be renamed as FilterElements too if they have that in their name.

  • CsvFilter
  • IntFilter
  • IntMatchFilter
  • IntRangeFilter
  • SuppressElement

@rnveach rnveach changed the title Add new interface SubFilter Rename SubFilters to FilterElement Nov 19, 2017

@rnveach rnveach changed the title Rename SubFilters to FilterElement Rename Filters which aren't module filters to FilterElement Nov 19, 2017

@checkstyle checkstyle deleted a comment from rnveach Nov 19, 2017

@checkstyle checkstyle deleted a comment from rnveach Nov 19, 2017

@checkstyle checkstyle deleted a comment from rnveach Nov 19, 2017

@checkstyle checkstyle deleted a comment from rnveach Nov 19, 2017

@checkstyle checkstyle deleted a comment from rnveach Nov 19, 2017

@romani romani added the approved label Nov 19, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Feb 18, 2019

@rnveach

This comment has been minimized.

Copy link
Member Author

rnveach commented Feb 18, 2019

XpathFilter was added to the list.

rnveach added a commit to rnveach/checkstyle that referenced this issue Feb 18, 2019

rnveach added a commit to rnveach/checkstyle that referenced this issue Feb 18, 2019

rnveach added a commit to rnveach/checkstyle that referenced this issue Feb 18, 2019

rnveach added a commit that referenced this issue Feb 18, 2019

@rnveach

This comment has been minimized.

Copy link
Member Author

rnveach commented Feb 18, 2019

@romani Issue is done, but some FilterElements still implement Filter.
Do we want to make a change for this? Issue doesn't declare this really.

@rnveach rnveach added this to the 8.18 milestone Feb 18, 2019

@romani

This comment has been minimized.

Copy link
Member

romani commented Feb 18, 2019

I do not think it is required. I think name is ok.

@romani romani closed this Feb 18, 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.