Hi Benson and welcome aboard!

A have couple of notes on this commit.

1. Please try to limit your commits to only one issue per commit. It
makes it easier for the rest of us to review the commits.

2. Please use the following form for the commit messages:

[<issueId>] <issueSummary>

for example:

[MCHANGES-254] Example doesn't work - spaces not allowed in statusIds
and resolutionIds after a comma

3. For some code specific comments see below...


On 2011-06-04 17:17, [email protected] wrote:
> Author: bimargulies
> Date: Sat Jun  4 15:17:46 2011
> New Revision: 1131414
> 
> URL: http://svn.apache.org/viewvc?rev=1131414&view=rev
> Log:
> [MCHANGES-254, MCHANGES-253] add more calls to trim, fix doc.
> 
> Also fix some warnings in site generation.
> 
> Modified:
>     maven/plugins/trunk/maven-changes-plugin/pom.xml
>     
> maven/plugins/trunk/maven-changes-plugin/src/main/java/org/apache/maven/plugin/jira/AbstractJiraDownloader.java
>     
> maven/plugins/trunk/maven-changes-plugin/src/main/java/org/apache/maven/plugin/jira/JiraXML.java
>     
> maven/plugins/trunk/maven-changes-plugin/src/site/apt/examples/customizing-jira-report.apt.vm
>     maven/plugins/trunk/maven-changes-plugin/src/site/apt/usage.apt.vm
> 
> Modified: maven/plugins/trunk/maven-changes-plugin/pom.xml
> URL: 
> http://svn.apache.org/viewvc/maven/plugins/trunk/maven-changes-plugin/pom.xml?rev=1131414&r1=1131413&r2=1131414&view=diff
> ==============================================================================
> --- maven/plugins/trunk/maven-changes-plugin/pom.xml (original)
> +++ maven/plugins/trunk/maven-changes-plugin/pom.xml Sat Jun  4 15:17:46 2011
> @@ -484,7 +484,7 @@ under the License.
>                
> <columnNames>Type,Key,Summary,Assignee,Status,Resolution,Created</columnNames>
>                <maxEntries>200</maxEntries>
>                <onlyCurrentVersion>true</onlyCurrentVersion>
> -              <resolutionIds>Closed</resolutionIds>
> +              <resolutionIds>Fixed</resolutionIds>
>                <sortColumnNames>Type,Key</sortColumnNames>
>              </configuration>
>              <reportSets>
> 
> Modified: 
> maven/plugins/trunk/maven-changes-plugin/src/main/java/org/apache/maven/plugin/jira/AbstractJiraDownloader.java
> URL: 
> http://svn.apache.org/viewvc/maven/plugins/trunk/maven-changes-plugin/src/main/java/org/apache/maven/plugin/jira/AbstractJiraDownloader.java?rev=1131414&r1=1131413&r2=1131414&view=diff
> ==============================================================================
> --- 
> maven/plugins/trunk/maven-changes-plugin/src/main/java/org/apache/maven/plugin/jira/AbstractJiraDownloader.java
>  (original)
> +++ 
> maven/plugins/trunk/maven-changes-plugin/src/main/java/org/apache/maven/plugin/jira/AbstractJiraDownloader.java
>  Sat Jun  4 15:17:46 2011
> @@ -31,6 +31,7 @@ import org.apache.commons.httpclient.Use
>  import org.apache.commons.httpclient.params.HttpClientParams;
>  import org.apache.commons.httpclient.auth.AuthScope;
>  import org.apache.commons.httpclient.methods.GetMethod;
> +import org.apache.maven.plugin.MojoExecutionException;
>  import org.apache.maven.plugin.issues.Issue;
>  import org.apache.maven.plugin.logging.Log;
>  import org.apache.maven.project.MavenProject;
> @@ -145,10 +146,10 @@ public abstract class AbstractJiraDownlo
>          if ( statusIds != null )
>          {
>              String[] stats = statusIds.split( "," );
> -
> -            for ( int i = 0; i < stats.length; i++ )
> +            for ( String stat : stats ) 
>              {
> -                String statusParam = statusMap.get( stats[i] );
> +                stat = stat.trim();
> +                String statusParam = statusMap.get( stats );
>  
>                  if ( statusParam != null )
>                  {
> @@ -162,9 +163,10 @@ public abstract class AbstractJiraDownlo
>          {
>              String[] prios = priorityIds.split( "," );
>  
> -            for ( int i = 0; i < prios.length; i++ )
> +            for ( String prio : prios ) 
>              {
> -                String priorityParam = priorityMap.get( prios[i] );
> +                prio = prio.trim();
> +                String priorityParam = priorityMap.get( prio );
>  
>                  if ( priorityParam != null )
>                  {
> @@ -178,9 +180,10 @@ public abstract class AbstractJiraDownlo
>          {
>              String[] resos = resolutionIds.split( "," );
>  
> -            for ( int i = 0; i < resos.length; i++ )
> +            for ( String reso : resos ) 
>              {
> -                String resoParam = resolutionMap.get( resos[i] );
> +                reso = reso.trim();
> +                String resoParam = resolutionMap.get( reso );
>  
>                  if ( resoParam != null )
>                  {
> @@ -194,11 +197,12 @@ public abstract class AbstractJiraDownlo
>          {
>              String[] components = component.split( "," );
>  
> -            for ( int i = 0; i < components.length; i++ )
> +            for ( String component : components ) 
>              {
> -                if ( components[i].length() > 0 )
> +                component = component.trim();
> +                if ( component.length() > 0 )
>                  {
> -                    localFilter.append( "&component=" ).append( 
> components[i] );
> +                    localFilter.append( "&component=" ).append( components );

Whoops, that should have been component, instead of components. I fixed
that in r1131484.

>                  }
>              }
>          }
> @@ -208,9 +212,9 @@ public abstract class AbstractJiraDownlo
>          {
>              String[] types = typeIds.split( "," );
>  
> -            for ( int i = 0; i < types.length; i++ )
> +            for ( String type : types )
>              {
> -                String typeParam = typeMap.get( types[i].trim() );
> +                String typeParam = typeMap.get( type.trim() );
>  
>                  if ( typeParam != null )
>                  {
> @@ -720,7 +724,7 @@ public abstract class AbstractJiraDownlo
>          }
>      }
>  
> -    public List<Issue> getIssueList()
> +    public List<Issue> getIssueList() throws MojoExecutionException
>      {
>          if ( output.isFile() )
>          {
> 
> Modified: 
> maven/plugins/trunk/maven-changes-plugin/src/main/java/org/apache/maven/plugin/jira/JiraXML.java
> URL: 
> http://svn.apache.org/viewvc/maven/plugins/trunk/maven-changes-plugin/src/main/java/org/apache/maven/plugin/jira/JiraXML.java?rev=1131414&r1=1131413&r2=1131414&view=diff
> ==============================================================================
> --- 
> maven/plugins/trunk/maven-changes-plugin/src/main/java/org/apache/maven/plugin/jira/JiraXML.java
>  (original)
> +++ 
> maven/plugins/trunk/maven-changes-plugin/src/main/java/org/apache/maven/plugin/jira/JiraXML.java
>  Sat Jun  4 15:17:46 2011
> @@ -30,6 +30,7 @@ import java.util.Locale;
>  import javax.xml.parsers.SAXParser;
>  import javax.xml.parsers.SAXParserFactory;
>  
> +import org.apache.maven.plugin.MojoExecutionException;
>  import org.apache.maven.plugin.issues.Issue;
>  import org.apache.maven.plugin.logging.Log;
>  import org.xml.sax.Attributes;
> @@ -90,15 +91,16 @@ public class JiraXML
>       * Parse the given xml file. The list of issues can then be retrieved 
> with {@link #getIssueList()}.
>       *
>       * @param xmlPath the file to pares.
> +     * @throws MojoExecutionException 
>       *
>       * @since 2.4
>       */
> -    public void parseXML( File xmlPath )
> +    public void parseXML( File xmlPath ) throws MojoExecutionException
>      {
>          parse( xmlPath );
>      }
>  
> -    private void parse( File xmlPath )
> +    private void parse( File xmlPath ) throws MojoExecutionException
>      {
>          try
>          {
> @@ -109,7 +111,7 @@ public class JiraXML
>          }
>          catch ( Throwable t )
>          {
> -            log.warn( t );
> +            throw new MojoExecutionException ( "Failed to parse JIRA XML.", 
> t );


Hmm, I'm trying to figure out why this exception was added. Can you
explain that? It doesn't seem to be related to the issues you are solving.

Since this is a utility class I'd prefer to not throw
MojoExecutionException. If an exception must be thrown can we use some
other Exception type instead, perhaps ParseException?


>          }
>      }
>  
> 
> Modified: 
> maven/plugins/trunk/maven-changes-plugin/src/site/apt/examples/customizing-jira-report.apt.vm
> URL: 
> http://svn.apache.org/viewvc/maven/plugins/trunk/maven-changes-plugin/src/site/apt/examples/customizing-jira-report.apt.vm?rev=1131414&r1=1131413&r2=1131414&view=diff
> ==============================================================================
> --- 
> maven/plugins/trunk/maven-changes-plugin/src/site/apt/examples/customizing-jira-report.apt.vm
>  (original)
> +++ 
> maven/plugins/trunk/maven-changes-plugin/src/site/apt/examples/customizing-jira-report.apt.vm
>  Sat Jun  4 15:17:46 2011
> @@ -162,7 +162,7 @@ Customizing the JIRA Report
>          <artifactId>maven-changes-plugin</artifactId>
>          <version>${project.version}</version>
>          <configuration>
> -          <resolutionIds>Closed</resolutionIds>
> +          <resolutionIds>Fixed</resolutionIds>
>            <statusIds>Resolved, Closed</statusIds>
>            <typeIds>Bug, New Feature, Improvement, Wish</typeIds>
>          </configuration>
> 
> Modified: maven/plugins/trunk/maven-changes-plugin/src/site/apt/usage.apt.vm
> URL: 
> http://svn.apache.org/viewvc/maven/plugins/trunk/maven-changes-plugin/src/site/apt/usage.apt.vm?rev=1131414&r1=1131413&r2=1131414&view=diff
> ==============================================================================
> --- maven/plugins/trunk/maven-changes-plugin/src/site/apt/usage.apt.vm 
> (original)
> +++ maven/plugins/trunk/maven-changes-plugin/src/site/apt/usage.apt.vm Sat 
> Jun  4 15:17:46 2011
> @@ -71,7 +71,7 @@ Usage
>  </document>
>  -------------------
>  
> - See the {{{changes.html}Changes Reference}} for details regarding the
> + See the {{{./changes.html}Changes Reference}} for details regarding the
>   <<<\<release\>>>> and <<<\<action\>>>> elements and their attributes.
>  
>   To generate the Changes Report, insert the Changes Plugin in the
> @@ -166,11 +166,11 @@ mvn site
>  *---------------+--------------------------------------------------------+
>  
>    If you use an issue management system other than the ones above, you need 
> to
> -  {{{changes-report-mojo.html#issueLinkTemplatePerSystem}configure an issue
> +  {{{./changes-report-mojo.html#issueLinkTemplatePerSystem}configure an issue
>    link template for it}}.
>    We would love to extend the table above with more issue management systems,
>    so if you have a working configuration that is not listed above, please 
> tell
> -  us about it by {{{issue-tracking.html}creating an issue for it}}.
> +  us about it by {{{./issue-tracking.html}creating an issue for it}}.
>  
>    <<Note:>> Make sure that your <<<\<issueManagement\>/\<url\>>>> is
>    correct. In particular, make sure that it has a trailing slash if it needs 
> one.
> @@ -231,7 +231,7 @@ mvn site
>  -------------------
>  
>    For info on how to modify the JIRA Report see the
> -  {{{examples/customizing-jira-report.html}Customizing the JIRA Report}}
> +  {{{./examples/customizing-jira-report.html}Customizing the JIRA Report}}
>    example.
>  
>  
> @@ -242,7 +242,7 @@ mvn site
>    announcement emails.
>  
>    For info on how to change the sender of the email see the
> -  {{{examples/specifying-mail-sender.html}Specifying the mail sender}} 
> example.
> +  {{{./examples/specifying-mail-sender.html}Specifying the mail sender}} 
> example.
>  
>  -------------------
>  <project>
> 
> 
> 


-- 
Dennis Lundberg

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to