Hi,

As I mentioned a few weeks ago, I'd like to clean up the way we do our
checkstyle rule checking. Right now we point the maven-checkstyle-report
plugin directly at a file in the svn repository.

Using svn directly does have the benefit of simplicity, and allows us to
update the rules easily. However it has the following disadvantages:
* prevents us from changing our repository layout without breaking old
releases.
* changing the rules changes the report generated when rebuilding old
releases
* cannot build maven site without network access to svn repo.

The alternative is to create a maven artifact containing the checkstyle
rules and deploy it to the repository. Then this artifact can be used by
the report plugin. This fixes all of the above. The only real
disadvantage is that to update the checkstyle rules we need to release a
new version of the rules artifact, then update the master pom. That's no
big deal though.

I have already checked in a checkstyle module here:
http://svn.apache.org/repos/asf/myfaces/myfaces-build-tools/trunk/other/checkstyle-rules/

The original checkstyle rules file had almost every check commented out;
in this module I have enabled the checks I think are reasonable. Note
that this module also holds tobago checkstyle rules, although I have no
idea whether the tobago team want to use this or not; this is mostly to
demonstrate that separate checkstyle rules *can* be in the same
checkstyle artifact if it is desired. Or can be overridden in a project,
just by redefining the maven-checkstyle-plugin configuration.

The patch below to the myfaces-master-pom would then switch over to
using this new module. Note that the checkstyle plugin is now configured
in <plugins> not <pluginManagement>. Using <pluginManagement> makes no
sense if we then reference the plugin in the reporting section of the
same pom.

Could you please indicate:
[+1]  yes, change the myfaces-master-pom to use checkstyle rules
artifact
[-1] no, leave things alone and remove the new checkstyle artifact

If people are happy with this, I will update the master pom, leave it to
settle in for a week or so, then call a vote to make a release of both
the rules artifact and a new master pom.

Thanks,
Simon


Index: pom.xml
===================================================================
--- pom.xml     (revision 660720)
+++ pom.xml     (working copy)
@@ -639,6 +639,20 @@
     <build>
         <defaultGoal>install</defaultGoal>
 
+        <plugins>
+          <plugin>
+            <artifactId>maven-checkstyle-plugin</artifactId>
+            <version>2.2</version>
+            <dependencies>
+              <dependency>
+                <groupId>org.apache.myfaces.buildtools</groupId>
+                <artifactId>checkstyle-rules</artifactId>
+                <version>1-SNAPSHOT</version>
+              </dependency>
+            </dependencies>
+          </plugin>
+        </plugins>
+
         <!-- 
           - The pluginManagement section does not declare actual
dependencies.
           - However if a child pom declares a dependency on one of the
plugins
@@ -685,11 +699,6 @@
               </plugin>
 
               <plugin>
-                <artifactId>maven-checkstyle-plugin</artifactId>
-                <version>2.1</version>
-              </plugin>
-
-              <plugin>
                 <artifactId>maven-javadoc-plugin</artifactId>
                 <version>2.3</version>
               </plugin>
@@ -762,17 +771,10 @@
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-checkstyle-plugin</artifactId>
-                <version>2.1</version>
+                <version>2.2</version>
                 <configuration>
-<!-- TODO: FIX THIS!
-  -
-  - Referencing resources directly from svn is very bad. Firstly, it
needs network access to build anything.
-  - But worse, if this pom is released with this here, then svn cannot
be reorganised to move these files
-  - without breaking any builds that use that released pom. Which means
the svn directory structure is
-  - effectively "locked" in place for years.
-  -->
-
<configLocation>http://svn.apache.org/repos/asf/myfaces/myfaces-master-pom/trunk/checkstyle/default/myfaces-checks.xml</configLocation>
-
<headerLocation>http://svn.apache.org/repos/asf/myfaces/myfaces-master-pom/trunk/checkstyle/default/myfaces-header.txt</headerLocation>
+
<configLocation>default/myfaces-checks.xml</configLocation>
+
<headerLocation>default/myfaces-header.txt</headerLocation>
                 </configuration>
             </plugin>
       </plugins>

Reply via email to