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 checkstyle_packages.xml from checkstyle #3660

Closed
romani opened this Issue Dec 16, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@romani
Member

romani commented Dec 16, 2016

We know where our classes are located, we should continue to support that in other jars

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Apr 3, 2017

Contributor

@romani
Will we need to resolve #3607 if we remove checkstyle_packages.xml ? Can we scan the directories which contain checks and do not rely on checkstyle_packages.xml?

Contributor

MEZk commented Apr 3, 2017

@romani
Will we need to resolve #3607 if we remove checkstyle_packages.xml ? Can we scan the directories which contain checks and do not rely on checkstyle_packages.xml?

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Apr 4, 2017

Member

@MEZk Our checkstyle_packages.xml is for only loading our checks. We know where our checks are by the hardcoded list we do in PackageObjectFactory. The file only remains to load 3rd party checks that don't write their own checkstyle_packages.xml and use our package names.
#3607 is to probably to load external, 3rd party checks which is why we need reflection.
So I don't think this issue relies on any other.

Member

rnveach commented Apr 4, 2017

@MEZk Our checkstyle_packages.xml is for only loading our checks. We know where our checks are by the hardcoded list we do in PackageObjectFactory. The file only remains to load 3rd party checks that don't write their own checkstyle_packages.xml and use our package names.
#3607 is to probably to load external, 3rd party checks which is why we need reflection.
So I don't think this issue relies on any other.

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Apr 4, 2017

Contributor

@rnveach
Now I see.

  1. We try to create a module based on NAME_TO_FULL_MODULE_NAME map which contains all fully qualified names of Checkstyle modules. (createObjectFromMap)
  2. If the previous attempt fails we try to create a module based on the information about packages taken from checkstyle_packages.xml.

The second attempt is for 3rd party checks as we know all Checkstyle modules when we try to perform the first attempt of the module creation.

Do I understand correctly?

Contributor

MEZk commented Apr 4, 2017

@rnveach
Now I see.

  1. We try to create a module based on NAME_TO_FULL_MODULE_NAME map which contains all fully qualified names of Checkstyle modules. (createObjectFromMap)
  2. If the previous attempt fails we try to create a module based on the information about packages taken from checkstyle_packages.xml.

The second attempt is for 3rd party checks as we know all Checkstyle modules when we try to perform the first attempt of the module creation.

Do I understand correctly?

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Apr 4, 2017

Member

@MEZk yes.

Member

rnveach commented Apr 4, 2017

@MEZk yes.

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

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

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

rnveach added a commit to rnveach/checkstyle that referenced this issue Aug 4, 2017

romani added a commit that referenced this issue Aug 5, 2017

@romani romani added this to the 8.2 milestone Aug 5, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Aug 5, 2017

Member

Fix is merged

Member

romani commented Aug 5, 2017

Fix is merged

@romani romani closed this Aug 5, 2017

@romani romani added the bug label Aug 8, 2017

soon added a commit to soon/checkstyle that referenced this issue Aug 29, 2017

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