Re: svn commit: r557062 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java

2007-08-01 Thread Paul King

Peter Reilly wrote:

On 7/17/07, Matt Benson <[EMAIL PROTECTED]> wrote:

The refactoring is much bigger than the formatting
here, FYI... wanted to reassure you that I am taking
your comments to heart, Peter.

Cool!
I think perhaps we should push to remove all
checkstyle errors for ant1.8. At work, I have
implemented a zero checkstyle error policy -
with an checkconfig based on ant's (removing some
of the sillier checks) - this is run as part of CI and
reports are seen by the project manager.
(within a couple of weeks all checkstyle errors
disappeared!).


Yes, I am a fan of breaking the build if a checkstyle
violation is found. And then you make your IDE know about
the exact same set of checkstyle rules, so any breakage
won't come as a surprise.

Paul.


Peter


-Matt

--- [EMAIL PROTECTED] wrote:

> Author: mbenson
> Date: Tue Jul 17 14:35:26 2007
> New Revision: 557062
>
> URL:
> http://svn.apache.org/viewvc?view=rev&rev=557062
> Log:
> fmt/refac
>
> Modified:
>
>
ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java
>
> Modified:
>
ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java
> URL:
>
http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java?view=diff&rev=557062&r1=557061&r2=557062 


>
== 


> ---
>
ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java
> (original)
> +++
>
ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java
> Tue Jul 17 14:35:26 2007
> @@ -76,8 +76,7 @@
>   * @param resource resource on classpath
>   */
>  public void setResource(String resource) {
> -assertSrcIsJavaResource();
> -((JavaResource) src).setName(resource);
> +
> getRequiredJavaResource().setName(resource);
>  }
>
>  /**
> @@ -100,8 +99,7 @@
>   * @param classpath to add to any existing
> classpath
>   */
>  public void setClasspath(Path classpath) {
> -assertSrcIsJavaResource();
> -((JavaResource)
> src).setClasspath(classpath);
> +
> getRequiredJavaResource().setClasspath(classpath);
>  }
>
>  /**
> @@ -109,8 +107,7 @@
>   * @return The classpath to be configured
>   */
>  public Path createClasspath() {
> -assertSrcIsJavaResource();
> -return ((JavaResource)
> src).createClasspath();
> +return
> getRequiredJavaResource().createClasspath();
>  }
>
>  /**
> @@ -119,8 +116,7 @@
>   * @param r The reference value
>   */
>  public void setClasspathRef(Reference r) {
> -assertSrcIsJavaResource();
> -((JavaResource) src).setClasspathRef(r);
> +
> getRequiredJavaResource().setClasspathRef(r);
>  }
>
>  /**
> @@ -128,8 +124,7 @@
>   * @return The classpath
>   */
>  public Path getClasspath() {
> -assertSrcIsJavaResource();
> -return ((JavaResource) src).getClasspath();
> +return
> getRequiredJavaResource().getClasspath();
>  }
>
>  /**
> @@ -150,7 +145,6 @@
>  }
>  throw new BuildException("Source
> resource does not exist: " + src);
>  }
> -
>  BufferedInputStream bis = null;
>  Reader instream = null;
>  ByteArrayInputStream tis = null;
> @@ -162,7 +156,6 @@
>  } else {
>  instream = new
> InputStreamReader(bis, encoding);
>  }
> -
>  ChainReaderHelper crh = new
> ChainReaderHelper();
>  crh.setPrimaryReader(instream);
>  crh.setFilterChains(filterChains);
> @@ -175,7 +168,6 @@
>  if (!text.endsWith("\n")) {
>  text = text + "\n";
>  }
> -
>  if (encoding == null) {
>  tis = new
> ByteArrayInputStream(text.getBytes());
>  } else {
> @@ -188,10 +180,8 @@
>  propertyTask.bindToOwner(this);
>  propertyTask.addProperties(props);
>  }
> -
>  } catch (final IOException ioe) {
> -final String message = "Unable to load
> file: " + ioe.toString();
> -throw new BuildException(message, ioe,
> getLocation());
> +throw new BuildException("Unable to
> load file: " + ioe, ioe, getLocation());
>  } finally {
>  FileUtils.close(bis);
>  FileUtils.close(tis);
> @@ -211,23 +201,24 @@
>   * @param a the resource to load as a single
> element Resource collection.
>   * @since Ant 1.7
>   */
> -public void addConfigured(ResourceCollection a)
> {
> +public synchronized void
> addConfigured(ResourceCollection a) {
>  if (src != null) {
>  throw new BuildException("only a single
> source is supported");
>  }
>  if (a.size() != 1) {
> -throw new BuildException("only single
> argument resource collections"
> -  

Re: svn commit: r557062 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java

2007-07-18 Thread Peter Reilly

On 7/17/07, Matt Benson <[EMAIL PROTECTED]> wrote:

The refactoring is much bigger than the formatting
here, FYI... wanted to reassure you that I am taking
your comments to heart, Peter.

Cool!
I think perhaps we should push to remove all
checkstyle errors for ant1.8. At work, I have
implemented a zero checkstyle error policy -
with an checkconfig based on ant's (removing some
of the sillier checks) - this is run as part of CI and
reports are seen by the project manager.
(within a couple of weeks all checkstyle errors
disappeared!).

Peter


-Matt

--- [EMAIL PROTECTED] wrote:

> Author: mbenson
> Date: Tue Jul 17 14:35:26 2007
> New Revision: 557062
>
> URL:
> http://svn.apache.org/viewvc?view=rev&rev=557062
> Log:
> fmt/refac
>
> Modified:
>
>
ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java
>
> Modified:
>
ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java
> URL:
>
http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java?view=diff&rev=557062&r1=557061&r2=557062
>
==
> ---
>
ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java
> (original)
> +++
>
ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java
> Tue Jul 17 14:35:26 2007
> @@ -76,8 +76,7 @@
>   * @param resource resource on classpath
>   */
>  public void setResource(String resource) {
> -assertSrcIsJavaResource();
> -((JavaResource) src).setName(resource);
> +
> getRequiredJavaResource().setName(resource);
>  }
>
>  /**
> @@ -100,8 +99,7 @@
>   * @param classpath to add to any existing
> classpath
>   */
>  public void setClasspath(Path classpath) {
> -assertSrcIsJavaResource();
> -((JavaResource)
> src).setClasspath(classpath);
> +
> getRequiredJavaResource().setClasspath(classpath);
>  }
>
>  /**
> @@ -109,8 +107,7 @@
>   * @return The classpath to be configured
>   */
>  public Path createClasspath() {
> -assertSrcIsJavaResource();
> -return ((JavaResource)
> src).createClasspath();
> +return
> getRequiredJavaResource().createClasspath();
>  }
>
>  /**
> @@ -119,8 +116,7 @@
>   * @param r The reference value
>   */
>  public void setClasspathRef(Reference r) {
> -assertSrcIsJavaResource();
> -((JavaResource) src).setClasspathRef(r);
> +
> getRequiredJavaResource().setClasspathRef(r);
>  }
>
>  /**
> @@ -128,8 +124,7 @@
>   * @return The classpath
>   */
>  public Path getClasspath() {
> -assertSrcIsJavaResource();
> -return ((JavaResource) src).getClasspath();
> +return
> getRequiredJavaResource().getClasspath();
>  }
>
>  /**
> @@ -150,7 +145,6 @@
>  }
>  throw new BuildException("Source
> resource does not exist: " + src);
>  }
> -
>  BufferedInputStream bis = null;
>  Reader instream = null;
>  ByteArrayInputStream tis = null;
> @@ -162,7 +156,6 @@
>  } else {
>  instream = new
> InputStreamReader(bis, encoding);
>  }
> -
>  ChainReaderHelper crh = new
> ChainReaderHelper();
>  crh.setPrimaryReader(instream);
>  crh.setFilterChains(filterChains);
> @@ -175,7 +168,6 @@
>  if (!text.endsWith("\n")) {
>  text = text + "\n";
>  }
> -
>  if (encoding == null) {
>  tis = new
> ByteArrayInputStream(text.getBytes());
>  } else {
> @@ -188,10 +180,8 @@
>  propertyTask.bindToOwner(this);
>  propertyTask.addProperties(props);
>  }
> -
>  } catch (final IOException ioe) {
> -final String message = "Unable to load
> file: " + ioe.toString();
> -throw new BuildException(message, ioe,
> getLocation());
> +throw new BuildException("Unable to
> load file: " + ioe, ioe, getLocation());
>  } finally {
>  FileUtils.close(bis);
>  FileUtils.close(tis);
> @@ -211,23 +201,24 @@
>   * @param a the resource to load as a single
> element Resource collection.
>   * @since Ant 1.7
>   */
> -public void addConfigured(ResourceCollection a)
> {
> +public synchronized void
> addConfigured(ResourceCollection a) {
>  if (src != null) {
>  throw new BuildException("only a single
> source is supported");
>  }
>  if (a.size() != 1) {
> -throw new BuildException("only single
> argument resource collections"
> - + " are
> supported");
> +throw new BuildException(
> +"only single-element resource
> collections are supported");
>  }
>  src = (Resource) a.iterator().next();
>  }
>

Re: svn commit: r557062 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java

2007-07-17 Thread Matt Benson
The refactoring is much bigger than the formatting
here, FYI... wanted to reassure you that I am taking
your comments to heart, Peter.

-Matt

--- [EMAIL PROTECTED] wrote:

> Author: mbenson
> Date: Tue Jul 17 14:35:26 2007
> New Revision: 557062
> 
> URL:
> http://svn.apache.org/viewvc?view=rev&rev=557062
> Log:
> fmt/refac
> 
> Modified:
>
>
ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java
> 
> Modified:
>
ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java
> URL:
>
http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java?view=diff&rev=557062&r1=557061&r2=557062
>
==
> ---
>
ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java
> (original)
> +++
>
ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java
> Tue Jul 17 14:35:26 2007
> @@ -76,8 +76,7 @@
>   * @param resource resource on classpath
>   */
>  public void setResource(String resource) {
> -assertSrcIsJavaResource();
> -((JavaResource) src).setName(resource);
> +   
> getRequiredJavaResource().setName(resource);
>  }
>  
>  /**
> @@ -100,8 +99,7 @@
>   * @param classpath to add to any existing
> classpath
>   */
>  public void setClasspath(Path classpath) {
> -assertSrcIsJavaResource();
> -((JavaResource)
> src).setClasspath(classpath);
> +   
> getRequiredJavaResource().setClasspath(classpath);
>  }
>  
>  /**
> @@ -109,8 +107,7 @@
>   * @return The classpath to be configured
>   */
>  public Path createClasspath() {
> -assertSrcIsJavaResource();
> -return ((JavaResource)
> src).createClasspath();
> +return
> getRequiredJavaResource().createClasspath();
>  }
>  
>  /**
> @@ -119,8 +116,7 @@
>   * @param r The reference value
>   */
>  public void setClasspathRef(Reference r) {
> -assertSrcIsJavaResource();
> -((JavaResource) src).setClasspathRef(r);
> +   
> getRequiredJavaResource().setClasspathRef(r);
>  }
>  
>  /**
> @@ -128,8 +124,7 @@
>   * @return The classpath
>   */
>  public Path getClasspath() {
> -assertSrcIsJavaResource();
> -return ((JavaResource) src).getClasspath();
> +return
> getRequiredJavaResource().getClasspath();
>  }
>  
>  /**
> @@ -150,7 +145,6 @@
>  }
>  throw new BuildException("Source
> resource does not exist: " + src);
>  }
> -
>  BufferedInputStream bis = null;
>  Reader instream = null;
>  ByteArrayInputStream tis = null;
> @@ -162,7 +156,6 @@
>  } else {
>  instream = new
> InputStreamReader(bis, encoding);
>  }
> -
>  ChainReaderHelper crh = new
> ChainReaderHelper();
>  crh.setPrimaryReader(instream);
>  crh.setFilterChains(filterChains);
> @@ -175,7 +168,6 @@
>  if (!text.endsWith("\n")) {
>  text = text + "\n";
>  }
> -
>  if (encoding == null) {
>  tis = new
> ByteArrayInputStream(text.getBytes());
>  } else {
> @@ -188,10 +180,8 @@
>  propertyTask.bindToOwner(this);
>  propertyTask.addProperties(props);
>  }
> -
>  } catch (final IOException ioe) {
> -final String message = "Unable to load
> file: " + ioe.toString();
> -throw new BuildException(message, ioe,
> getLocation());
> +throw new BuildException("Unable to
> load file: " + ioe, ioe, getLocation());
>  } finally {
>  FileUtils.close(bis);
>  FileUtils.close(tis);
> @@ -211,23 +201,24 @@
>   * @param a the resource to load as a single
> element Resource collection.
>   * @since Ant 1.7
>   */
> -public void addConfigured(ResourceCollection a)
> {
> +public synchronized void
> addConfigured(ResourceCollection a) {
>  if (src != null) {
>  throw new BuildException("only a single
> source is supported");
>  }
>  if (a.size() != 1) {
> -throw new BuildException("only single
> argument resource collections"
> - + " are
> supported");
> +throw new BuildException(
> +"only single-element resource
> collections are supported");
>  }
>  src = (Resource) a.iterator().next();
>  }
>  
> -private void assertSrcIsJavaResource() {
> +private synchronized JavaResource
> getRequiredJavaResource() {
>  if (src == null) {
>  src = new JavaResource();
>  src.setProject(getProject());
>  } else if (!(src instanceof JavaResource))
> {
>  throw new BuildException("expected a
> java resource as source")

svn commit: r557062 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java

2007-07-17 Thread mbenson
Author: mbenson
Date: Tue Jul 17 14:35:26 2007
New Revision: 557062

URL: http://svn.apache.org/viewvc?view=rev&rev=557062
Log:
fmt/refac

Modified:
ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java

Modified: 
ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java
URL: 
http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java?view=diff&rev=557062&r1=557061&r2=557062
==
--- ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java 
(original)
+++ ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java 
Tue Jul 17 14:35:26 2007
@@ -76,8 +76,7 @@
  * @param resource resource on classpath
  */
 public void setResource(String resource) {
-assertSrcIsJavaResource();
-((JavaResource) src).setName(resource);
+getRequiredJavaResource().setName(resource);
 }
 
 /**
@@ -100,8 +99,7 @@
  * @param classpath to add to any existing classpath
  */
 public void setClasspath(Path classpath) {
-assertSrcIsJavaResource();
-((JavaResource) src).setClasspath(classpath);
+getRequiredJavaResource().setClasspath(classpath);
 }
 
 /**
@@ -109,8 +107,7 @@
  * @return The classpath to be configured
  */
 public Path createClasspath() {
-assertSrcIsJavaResource();
-return ((JavaResource) src).createClasspath();
+return getRequiredJavaResource().createClasspath();
 }
 
 /**
@@ -119,8 +116,7 @@
  * @param r The reference value
  */
 public void setClasspathRef(Reference r) {
-assertSrcIsJavaResource();
-((JavaResource) src).setClasspathRef(r);
+getRequiredJavaResource().setClasspathRef(r);
 }
 
 /**
@@ -128,8 +124,7 @@
  * @return The classpath
  */
 public Path getClasspath() {
-assertSrcIsJavaResource();
-return ((JavaResource) src).getClasspath();
+return getRequiredJavaResource().getClasspath();
 }
 
 /**
@@ -150,7 +145,6 @@
 }
 throw new BuildException("Source resource does not exist: " + src);
 }
-
 BufferedInputStream bis = null;
 Reader instream = null;
 ByteArrayInputStream tis = null;
@@ -162,7 +156,6 @@
 } else {
 instream = new InputStreamReader(bis, encoding);
 }
-
 ChainReaderHelper crh = new ChainReaderHelper();
 crh.setPrimaryReader(instream);
 crh.setFilterChains(filterChains);
@@ -175,7 +168,6 @@
 if (!text.endsWith("\n")) {
 text = text + "\n";
 }
-
 if (encoding == null) {
 tis = new ByteArrayInputStream(text.getBytes());
 } else {
@@ -188,10 +180,8 @@
 propertyTask.bindToOwner(this);
 propertyTask.addProperties(props);
 }
-
 } catch (final IOException ioe) {
-final String message = "Unable to load file: " + ioe.toString();
-throw new BuildException(message, ioe, getLocation());
+throw new BuildException("Unable to load file: " + ioe, ioe, 
getLocation());
 } finally {
 FileUtils.close(bis);
 FileUtils.close(tis);
@@ -211,23 +201,24 @@
  * @param a the resource to load as a single element Resource collection.
  * @since Ant 1.7
  */
-public void addConfigured(ResourceCollection a) {
+public synchronized void addConfigured(ResourceCollection a) {
 if (src != null) {
 throw new BuildException("only a single source is supported");
 }
 if (a.size() != 1) {
-throw new BuildException("only single argument resource 
collections"
- + " are supported");
+throw new BuildException(
+"only single-element resource collections are supported");
 }
 src = (Resource) a.iterator().next();
 }
 
-private void assertSrcIsJavaResource() {
+private synchronized JavaResource getRequiredJavaResource() {
 if (src == null) {
 src = new JavaResource();
 src.setProject(getProject());
 } else if (!(src instanceof JavaResource)) {
 throw new BuildException("expected a java resource as source");
 }
+return (JavaResource) src;
 }
 }



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]