maven-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dennis Lundberg <denn...@apache.org>
Subject Re: svn commit: r1131414 - in /maven/plugins/trunk/maven-changes-plugin: ./ src/main/java/org/apache/maven/plugin/jira/ src/site/apt/ src/site/apt/examples/
Date Sat, 04 Jun 2011 20:04:37 GMT
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, bimargulies@apache.org 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: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Mime
View raw message