[GitHub] rnveach commented on issue #5: [MCHECKSTYLE-353] - Don't resolve any dependencies

[GitBox <gi...@apache.org>]

rnveach commented on issue #5: [MCHECKSTYLE-353] - Don't resolve any dependencies
URL: https://github.com/apache/maven-checkstyle-plugin/pull/5#issuecomment-460088347
 
 
   > does checkstyle need dependency resolution ?
   
   I'm not fully sure what dependency resolution is.
   It sounds like you mean breaking outside checkstyle's self-contained code to see what other dependencies/classes/jars are added to the classpath, right?
   Does this mean without dependency resolution reflection, classpath scanning, and other methods can't find those classes/resources?
   If so, then yes it is needed. See below for all the cases I can think of.
   
   > I think that there is a way to load the configuration by loading it from the classpath
   
   Yes, you can load configurations and and other files (suppressions, etc..) found in other dependencies/JARs like a resource. We specifically have some pre-made configurations in our JARs like google and sun. If people want to follow those styles we tell them to use the ones contained within the checkstyle itself instead of copying/pasting it. This way when they upgrade checkstyle those configurations are automatically upgraded. I have seen one group put their own configuration into a separate dependency for this same purpose. There are other files loaded this way like suppressions, import control, package names (used for next section), etc...
   See https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/utils/CommonUtil.java#L505 .
   
   We also allow users to write their own 3rd party checks for use. They put them in the configuration by name only and we find it by scanning the entire classpath. There is one repository that contains 3rd party checks like [sevntu](https://github.com/sevntu-checkstyle/sevntu.checkstyle).
   See https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/PackageObjectFactory.java#L295
   and https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/utils/ModuleReflectionUtil.java#L56
   
   > this change should break this feature but we aren't breaking any integration test on the maven plugin.
   
   Do you have any tests using the mentioned above?
   Example using embedded config inside checkstyle's JAR: https://github.com/checkstyle/checkstyle/blob/master/src/test/java/com/puppycrawl/tools/checkstyle/MainTest.java#L272
   Examples of extra file loaded as resource: http://checkstyle.sourceforge.net/config_imports.html#ImportControl
   For 3rd party checks, I mentioned sevntu above.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services