I looked over the code from two perspectives: correctness and future extensibility. My comments are below.
> -----Original Message----- > From: Ingmar J Stein [mailto:[EMAIL PROTECTED] > 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:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
