You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by dk...@apache.org on 2010/06/03 15:28:58 UTC

svn commit: r950989 - /maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java

Author: dkulp
Date: Thu Jun  3 13:28:57 2010
New Revision: 950989

URL: http://svn.apache.org/viewvc?rev=950989&view=rev
Log:
Fix the checkstyle it tests.
This is really a complete hack to support MCHECKSTYLE-131 which, IMO, should not be supported.   Just because it worked at one point despite not falling into the documented and supported use cases does not, to me, mean we should really support it.

Modified:
    maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java

Modified: maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java
URL: http://svn.apache.org/viewvc/maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java?rev=950989&r1=950988&r2=950989&view=diff
==============================================================================
--- maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java (original)
+++ maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java Thu Jun  3 13:28:57 2010
@@ -31,6 +31,7 @@ import java.util.List;
 import java.util.Properties;
 
 import org.apache.maven.artifact.DependencyResolutionRequiredException;
+import org.apache.maven.project.MavenProject;
 import org.codehaus.plexus.logging.AbstractLogEnabled;
 import org.codehaus.plexus.resource.ResourceManager;
 import org.codehaus.plexus.resource.loader.FileResourceCreationException;
@@ -454,12 +455,16 @@ public class DefaultCheckstyleExecutor
             {
                 getLogger().debug( "request.getConfigLocation() " + request.getConfigLocation() );
             }
-            File parent = request.getProject().getFile().getParentFile();
-            if (parent != null)
-            {
-                // MCHECKSTYLE-131 ( olamy ) I don't like this hack. what's happened if this is defined in parent/parent pom
-                // it will breaks
-                locator.addSearchPath( FileResourceLoader.ID, request.getProject().getFile().getParentFile().getAbsolutePath() );
+            
+            MavenProject parent = request.getProject();
+            while ( parent != null && parent.getFile() != null )
+            {
+                // MCHECKSTYLE-131 ( olamy ) I don't like this hack.
+                // (dkulp) Me either.   It really pollutes the location stuff
+                // by allowing searches of stuff outside the current module.
+                File dir = parent.getFile().getParentFile();
+                locator.addSearchPath( FileResourceLoader.ID, dir.getAbsolutePath() );
+                parent = parent.getParent();
             }
             locator.addSearchPath( "url", "" );
 



Re: svn commit: r950989 - /maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java

Posted by Benjamin Bentmann <be...@udo.edu>.
Olivier Lamy wrote:

> We are two who don't like this hack :-)
> So what's about don't support this ?

+1, sharing of resources between modules should happen via a resource 
bundle that can be put onto the plugin's class path.


Benjamin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: svn commit: r950989 - /maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java

Posted by nicolas de loof <ni...@gmail.com>.
+1 to remove this "feature".
A module is expected to build either with it's parent POM in parent
directory or by donwloading it from repo, so relying on such *
search-in-parent-folder* feature is a bad practice.

Nicolas

2010/6/3 Olivier Lamy <ol...@apache.org>

> Hi,
> We are two who don't like this hack :-)
> So what's about don't support this ?
> Others ?
>
> 2010/6/3  <dk...@apache.org>:
> > Author: dkulp
> > Date: Thu Jun  3 13:28:57 2010
> > New Revision: 950989
> >
> > URL: http://svn.apache.org/viewvc?rev=950989&view=rev
> > Log:
> > Fix the checkstyle it tests.
> > This is really a complete hack to support MCHECKSTYLE-131 which, IMO,
> should not be supported.   Just because it worked at one point despite not
> falling into the documented and supported use cases does not, to me, mean we
> should really support it.
> >
> > Modified:
> >
>  maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java
> >
> > Modified:
> maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java
> > URL:
> http://svn.apache.org/viewvc/maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java?rev=950989&r1=950988&r2=950989&view=diff
> >
> ==============================================================================
> > ---
> maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java
> (original)
> > +++
> maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java
> Thu Jun  3 13:28:57 2010
> > @@ -31,6 +31,7 @@ import java.util.List;
> >  import java.util.Properties;
> >
> >  import org.apache.maven.artifact.DependencyResolutionRequiredException;
> > +import org.apache.maven.project.MavenProject;
> >  import org.codehaus.plexus.logging.AbstractLogEnabled;
> >  import org.codehaus.plexus.resource.ResourceManager;
> >  import
> org.codehaus.plexus.resource.loader.FileResourceCreationException;
> > @@ -454,12 +455,16 @@ public class DefaultCheckstyleExecutor
> >             {
> >                 getLogger().debug( "request.getConfigLocation() " +
> request.getConfigLocation() );
> >             }
> > -            File parent =
> request.getProject().getFile().getParentFile();
> > -            if (parent != null)
> > -            {
> > -                // MCHECKSTYLE-131 ( olamy ) I don't like this hack.
> what's happened if this is defined in parent/parent pom
> > -                // it will breaks
> > -                locator.addSearchPath( FileResourceLoader.ID,
> request.getProject().getFile().getParentFile().getAbsolutePath() );
> > +
> > +            MavenProject parent = request.getProject();
> > +            while ( parent != null && parent.getFile() != null )
> > +            {
> > +                // MCHECKSTYLE-131 ( olamy ) I don't like this hack.
> > +                // (dkulp) Me either.   It really pollutes the location
> stuff
> > +                // by allowing searches of stuff outside the current
> module.
> > +                File dir = parent.getFile().getParentFile();
> > +                locator.addSearchPath( FileResourceLoader.ID,
> dir.getAbsolutePath() );
> > +                parent = parent.getParent();
> >             }
> >             locator.addSearchPath( "url", "" );
> >
> >
> >
> >
>
>
>
> --
> Olivier
> http://twitter.com/olamy
> http://fr.linkedin.com/in/olamy
> http://www.viadeo.com/fr/profile/olivier.lamy7
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>

Re: svn commit: r950989 - /maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java

Posted by Olivier Lamy <ol...@apache.org>.
Hi,
We are two who don't like this hack :-)
So what's about don't support this ?
Others ?

2010/6/3  <dk...@apache.org>:
> Author: dkulp
> Date: Thu Jun  3 13:28:57 2010
> New Revision: 950989
>
> URL: http://svn.apache.org/viewvc?rev=950989&view=rev
> Log:
> Fix the checkstyle it tests.
> This is really a complete hack to support MCHECKSTYLE-131 which, IMO, should not be supported.   Just because it worked at one point despite not falling into the documented and supported use cases does not, to me, mean we should really support it.
>
> Modified:
>    maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java
>
> Modified: maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java
> URL: http://svn.apache.org/viewvc/maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java?rev=950989&r1=950988&r2=950989&view=diff
> ==============================================================================
> --- maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java (original)
> +++ maven/plugins/trunk/maven-checkstyle-plugin/src/main/java/org/apache/maven/plugin/checkstyle/DefaultCheckstyleExecutor.java Thu Jun  3 13:28:57 2010
> @@ -31,6 +31,7 @@ import java.util.List;
>  import java.util.Properties;
>
>  import org.apache.maven.artifact.DependencyResolutionRequiredException;
> +import org.apache.maven.project.MavenProject;
>  import org.codehaus.plexus.logging.AbstractLogEnabled;
>  import org.codehaus.plexus.resource.ResourceManager;
>  import org.codehaus.plexus.resource.loader.FileResourceCreationException;
> @@ -454,12 +455,16 @@ public class DefaultCheckstyleExecutor
>             {
>                 getLogger().debug( "request.getConfigLocation() " + request.getConfigLocation() );
>             }
> -            File parent = request.getProject().getFile().getParentFile();
> -            if (parent != null)
> -            {
> -                // MCHECKSTYLE-131 ( olamy ) I don't like this hack. what's happened if this is defined in parent/parent pom
> -                // it will breaks
> -                locator.addSearchPath( FileResourceLoader.ID, request.getProject().getFile().getParentFile().getAbsolutePath() );
> +
> +            MavenProject parent = request.getProject();
> +            while ( parent != null && parent.getFile() != null )
> +            {
> +                // MCHECKSTYLE-131 ( olamy ) I don't like this hack.
> +                // (dkulp) Me either.   It really pollutes the location stuff
> +                // by allowing searches of stuff outside the current module.
> +                File dir = parent.getFile().getParentFile();
> +                locator.addSearchPath( FileResourceLoader.ID, dir.getAbsolutePath() );
> +                parent = parent.getParent();
>             }
>             locator.addSearchPath( "url", "" );
>
>
>
>



-- 
Olivier
http://twitter.com/olamy
http://fr.linkedin.com/in/olamy
http://www.viadeo.com/fr/profile/olivier.lamy7

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org