ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Steve Cohen <sco...@javactivity.org>
Subject Re: [patch] FTP.java - adding support for new features in commons-net 1.4.0 and performance improvement
Date Wed, 11 May 2005 11:37:57 GMT
Neeme Praks wrote:
> 
> Steve Loughran wrote:
> 
>>
>> I worry about releasing any change to code without giving it time to 
>> stabilise and beta test. Last minute "this won't break anything" 
>> patches always break something. Always. At least in my experience.
>>
>> If commons1.4.0 is incompatible with shipping <ftp> then yes, we have 
>> no choice but to upgrade. But if it is a feature enhancement, then it 
>> needs
>> to go into CVS_HEAD
> 
> 
> Very legitimate concern.
> However, this is a trivial change.
> commons-net 1.4.0 added a configuration javabean for FTP client.
> It is a simple value-object that has some setters and then it can be 
> used to configure a FTP client instance.
> 
> All the added code does is expose this javabean on the FTP task through 
> delegating setters.
> 
> Let me illustrate with some code:
> 
> public class FTP extends Task {
> [...]
>     private FTPClientConfig configuration = null;
> 
>     /**
>      * Gets a FTPClientConfig. If the configuration object has not been
>      * created yet, it is created also.
>      */
>     private FTPClientConfig getConfiguration() {
>         if (this.configuration == null) {
>             this.configuration = new FTPClientConfig();
>         }
>         return this.configuration;
>     }
> 
>     /**
>      * Delegate method for 
> <code>FTPClientConfig.setDefaultDateFormatStr(String)</code>.
>      * @param defaultDateFormatStr
>      * @see #getConfiguration()
>      * @see FTPClientConfig
>      */
>     public void setDefaultDateFormat(String defaultDateFormatStr) {
>         getConfiguration()
>             .setDefaultDateFormatStr(defaultDateFormatStr);
>     }
> 
> [...there are 5 more delegating setters like this, but I'm skipping them 
> here for clarity...]
> 
>     public void execute() throws BuildException {
>         [...]
>         ftp = new FTPClient();
>         if (this.configuration != null) {
>             ftp.configure(this.configuration);
>         }
>         ftp.connect(server, port);
>         [...]
>     }
> 
> [...]
> }
> 
> Simple enough, no?
> Assuming that commons-net code is bug-free, this code cannot possibly 
> break anything. And it is backwards compatible, if there is no 
> configuration set, no configuration is used.
> 
> Rgds,
> Neeme
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
> 
> 
> 
Well, you're both right.  The new commons-net code is not incompatible 
with Ant.  This is just a new feature of commons-net.

However, it's an extremely simple feature, just passing bean attributes 
around.  I am quite confident that we can add just the support for the 
new commons-net feature and have all the tests pass.

Other parts of Mr. Praks' patch are less simple (retry handler, etc.) 
and I've asked him to remove those for now.  These definitely belong in 
CVS_HEAD after the release.


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


Mime
View raw message