tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Barker" <billwbar...@verizon.net>
Subject Re: svn commit: r966882 - in /tomcat/trunk: build.properties.default build.xml checkstyle.xml webapps/docs/changelog.xml
Date Sat, 24 Jul 2010 03:05:14 GMT


"Bill Barker" <billwbarker@verizon.net> wrote in message 
news:i2dje8$es0$1@dough.gmane.org...
>
>
> <markt@apache.org> wrote in message 
> news:20100722223130.A183D23889E0@eris.apache.org...
>> Author: markt
>> Date: Thu Jul 22 22:31:30 2010
>> New Revision: 966882
>>
>> URL: http://svn.apache.org/viewvc?rev=966882&view=rev
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49268
>> Add necessary plumbing to enable Checkstyle
>> The config file is deliberately empty. The check will be uncommented once 
>> the source code has been fixed (~200 files contain tabs).
>>
>> Added:
>>    tomcat/trunk/checkstyle.xml   (with props)
>> Modified:
>>    tomcat/trunk/build.properties.default
>>    tomcat/trunk/build.xml
>>    tomcat/trunk/webapps/docs/changelog.xml
>>
>> Modified: tomcat/trunk/build.properties.default
>> URL: 
>> http://svn.apache.org/viewvc/tomcat/trunk/build.properties.default?rev=966882&r1=966881&r2=966882&view=diff
>> ==============================================================================
>> --- tomcat/trunk/build.properties.default (original)
>> +++ tomcat/trunk/build.properties.default Thu Jul 22 22:31:30 2010
>> @@ -120,6 +120,12 @@ junit.lib=${junit.home}
>> junit.jar=${junit.lib}/junit.jar
>> junit.loc=${base-sf.loc}/junit/junit3.8.2.zip
>>
>> +# ----- Checkstyle, version 5.1 or later -----
>> +checkstyle.version=5.1
>> +checkstyle.home=${base.path}/checkstyle-${checkstyle.version}
>> +checkstyle.loc=${base-sf.loc}/checkstyle/checkstyle-${checkstyle.version}.zip
>> +checkstyle.jar=${checkstyle.home}/checkstyle-all-${checkstyle.version}.jar
>> +
>> # ----- JSON Libraries (for bayeux module) -----
>> json-lib.home=${base.path}/json-20080701
>> json-lib.lib=http://repo1.maven.org/maven2/org/json/json/20080701/json-20080701.jar
>>
>> Modified: tomcat/trunk/build.xml
>> URL: 
>> http://svn.apache.org/viewvc/tomcat/trunk/build.xml?rev=966882&r1=966881&r2=966882&view=diff
>> ==============================================================================
>> --- tomcat/trunk/build.xml (original)
>> +++ tomcat/trunk/build.xml Thu Jul 22 22:31:30 2010
>> @@ -410,7 +410,18 @@
>>
>>   </target>
>>
>> -  <target name="compile" depends="build-prepare,download-compile">
>> +  <target name="validate" depends="download-validate">
>> +    <taskdef resource="checkstyletask.properties"
>> +           classpath="${checkstyle.jar}" />
>> +  <checkstyle config="checkstyle.xml">
>> +      <fileset dir="java">
>> +      <include name="**/*.java" />
>> +        <exclude name="org/apache/tomcat/util/bcel/**" />
>> +    </fileset>
>> +  </checkstyle>
>> +  </target>
>> +
>> +  <target name="compile" 
>> depends="build-prepare,download-compile,validate">
>>
>
> I'm -1 (vote, not veto) on having validate be a dependency for compile. 
> Historically, Tomcat has been very lenient about what is required just to 
> build the project, and I can't see the reason to force checkstyle just to 
> be able to build.   Mostly my concern is with Gump (which I can fix the 
> metadata when svn comes up again).  But that puts a transitive dependency 
> on bcel that will get fixed who knows when.
>
> I'm more than happy to add a tomcat-trunk-validate project to Gump if it 
> is split from compile.
>

Having tried this (yes, I should have done this first), changing to -1 veto. 
In the OS world of cat herding, we should allow for the fact that somebody 
mistakenly violates the checkstyle rules and commits without building.  It 
happens all the time, and then they go off to sleep and nobody else can 
build the project with a fresh checkout for hours.

As I've said, I'm happy to add this to Gump (assuming that bcel ever builds 
ever again) if it a separate target to get regular automated notices.  But 
developers can run a separate target themselves (just like test).

>>     <!-- Compile internal server components -->
>>     <javac srcdir="java" destdir="${tomcat.classes}"
>> @@ -1888,6 +1899,17 @@ Apache Tomcat ${version} native binaries
>>
>>   <!-- ================ Download and dependency building 
>> =================== -->
>>
>> +  <target name="download-validate"
>> +          description="Download components necessary to validate source" 
>>  >
>> +
>> +    <antcall target="downloadzip">
>> +      <param name="sourcefile" value="${checkstyle.loc}"/>
>> +      <param name="destfile" value="${checkstyle.jar}"/>
>> +      <param name="destdir" value="${base.path}"/>
>> +    </antcall>
>> +
>> +  </target>
>> +
>>   <target name="download-compile"
>>        description="Download (and build) components necessary to compile" 
>>  >
>>
>>
>> Added: tomcat/trunk/checkstyle.xml
>> URL: 
>> http://svn.apache.org/viewvc/tomcat/trunk/checkstyle.xml?rev=966882&view=auto
>> ==============================================================================
>> --- tomcat/trunk/checkstyle.xml (added)
>> +++ tomcat/trunk/checkstyle.xml Thu Jul 22 22:31:30 2010
>> @@ -0,0 +1,25 @@
>> +<?xml version="1.0"?>
>> +<!--
>> + Licensed to the Apache Software Foundation (ASF) under one or more
>> +  contributor license agreements.  See the NOTICE file distributed with
>> +  this work for additional information regarding copyright ownership.
>> +  The ASF licenses this file to You under the Apache License, Version 
>> 2.0
>> +  (the "License"); you may not use this file except in compliance with
>> +  the License.  You may obtain a copy of the License at
>> +
>> +      http://www.apache.org/licenses/LICENSE-2.0
>> +
>> +  Unless required by applicable law or agreed to in writing, software
>> +  distributed under the License is distributed on an "AS IS" BASIS,
>> +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
>> implied.
>> +  See the License for the specific language governing permissions and
>> +  limitations under the License.
>> +-->
>> +<!DOCTYPE module PUBLIC
>> +    "-//Puppy Crawl//DTD Check Configuration 1.2//EN"
>> +    "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
>> +<module name="Checker">
>> +<!--
>> +  <module name="FileTabCharacter"/>
>> +-->
>> +</module>
>> \ No newline at end of file
>>
>> Propchange: tomcat/trunk/checkstyle.xml
>> ------------------------------------------------------------------------------
>>    svn:eol-style = native
>>
>> Modified: tomcat/trunk/webapps/docs/changelog.xml
>> URL: 
>> http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=966882&r1=966881&r2=966882&view=diff
>> ==============================================================================
>> --- tomcat/trunk/webapps/docs/changelog.xml (original)
>> +++ tomcat/trunk/webapps/docs/changelog.xml Thu Jul 22 22:31:30 2010
>> @@ -294,6 +294,11 @@
>>         Re-factor unit tests to enable them to be run once with each of 
>> the HTTP
>>         connector implementations (BIO, NIO and APR/native). (markt)
>>       </add>
>> +      <add>
>> +        <bug>49268</bug>: Add the necessary plumbing to include 
>> CheckStyle in
>> +        the build process. Start with no checks. Additional checks will 
>> be
>> +        added as they are agreed. (markt)
>> +      </add>
>>     </changelog>
>>   </subsection>
>> </section> 



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


Mime
View raw message