ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Albrecht, Matt" <matt.albre...@zilliant.com>
Subject RE: echoproperties
Date Tue, 07 May 2002 19:28:21 GMT
I looked over the code from two perspectives: correctness and future
extensibility.  My comments are below.

> -----Original Message-----
> From: Ingmar J Stein [mailto:stein@xtramind.com]
> Sent: Tuesday, May 07, 2002 2:15 AM
> To: Ant Developers List
> Subject: echoproperties
> 
> > The "srcfile" is an interesting addition.  I like it.  Will 
> there be a
> > future need/want to load the XML formatted properties here?
> 
> I don't see that need right now, but it might be a good idea to
> merge xmlproperties and echoproperties. I can imagine a single
> task that reads and writes property files in either text or 
> XML format.

In this case, should we have a "srcformat" and "destformat"?  At the very
least, at this stage of the game we could have "destformat", so we don't run
into confusing backwards-compatibility issues.  Unless you expect the task
to eventually auto-detect the source format...  I'm not volunteering :-)

> > the loading of the
> > Properties object from the project Hashtable relies on a JDK 1.2 API
> > (putAll).
> 
> Yeah, that's why it's an optional task, isn't it?

Stefan and I discussed this when I first submitted the task.  I originally
used "store", but we backed that out and now there's the reflection to
determine which method to invoke.  As Stefan said, the operation is simple
enough, that we may as well be JDK 1.1 compatible.  Same thing with putAll -
we can replace this with 5 lines of Java and do the same thing.

> Mmh, that's a matter of taste. Personally, I don't like to 
> regenerate static
> files every time I run a test, but if that's ant's test 
> writing policy, I
> will change
> it at once.

For me, it's just a matter of taste.  But I don't think there's a policy on
it.

> Ingmar
>

The tests look fine (if Stefan adds in his patch for the tests to remove the
space), and the error checking is very robust.  I did a bit of research to
see what it would take to split this so it did have the
one-class-per-formatter, and the only thing that pops out is the protected
methods that implement the writing: to maintain backwards binary
compatibility, they would be deprecated and simply reference the formatter
classes.  Since the JDK property writers are already protected, we can't do
much about that (unless someone wants to quickly patch Ant 1.5 beta), but
the XML formatting is new, so perhaps it should be "private".

Once the task is JDK 1.1 compatible, I'm a +1, for what it counts.


-Matt
Please excuse my poor English written.

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


Mime
View raw message