Re: svn commit: r557062 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/LoadProperties.java
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
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
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
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]