thanks for the thorough code review, jon are you signing up to commit this code, then? another question: is it ok to use java2-specific calls in ant?
Jon Skeet wrote: > > > I wrote a small ant task (called jad) that is useful for J2ME/MIDP > > development. This task updates the size of a MIDlet jar in the jad > > file. The jar size mentioned in the jad must be accurate or the MIDlet > > is rejected by Motorola's iDEN phones. Sun's J2ME Wireless Toolkit > > does this automatically, but this tool is useful for those who prefer > > to build MIDlets using ant (like myself :-)). > > Looks mostly okay to me. A few things: > > 1) Copyright - I don't think we'd be able to do anything without the ASF > being given the copyright and the licence changing appropriately. that will not be a problem > 2) We'd probably want to change the package to something like > org.apache.tools.ant.taskdefs.optional.j2me no problem > 3) Any reason why the jar is specified as a File but the jad is specified as > a String? after some testing i found that File works much better than String, so now both are Files. thanks > 4) I'd probably move tryExecute into the block of the try, personally, but > that's just a style issue. > > 5) I'd clean up closing readers/writers with finally blocks - currently they > could hang around. done > 6) Is a Jad file actually a Properties file? I can't remember off-hand... If > so, it's worth using the Properties.load and Properties.store methods rather > than reading it in directly. no, it is like a manifest. i too first thought of just using Properties, but Properties uses '=' as the separator, whereas JADs use ':' > 7) If it's not a actually a Properties file, is it at least meant to be in a > specific encoding? If so, it's worth specifying. the MIDP spec says that the JAD is in Unicode > 8) It's probably worth doing more "user friendly" parameter checking than > relying on IOExceptions - check for both files existing and the jad file > being writable before trying to do anything else. done > I'm a pretty new committer, so I don't know if there's anything else that > needs checking. Oh, and do you fancy writing a manual page for it? :) do you mean extras like javadocs, internationalization? i added synchronization... as to the manual page, i must write one, i suppose. i may not have time to do this before JavaOne, though. > Jon > > -- > To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> > For additional commands, e-mail: <mailto:[EMAIL PROTECTED]> -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
