Let me rephrase the question: couldn't we have two sentinel files instead of
four? Right now you have two sentinel files for dev/core and two for
distro-source; each could get by with only one sentinel file whose name
composited gwt.version with gwt.svnrev.filename.
Detailed review:
At a high level, I'm not sure why you went the route of making things so
general. Why does gwt.svninfo need to take parameters to say what props to
read the values into? Why not simply always read them into {gwt.svnrev} and
${gwt.svnrev.filename}? For that matter, why does gwt.revfilter need to
take parameters for those values? I think both of these should use
hardcoded property names; the other advantage to this is the ability to
override these values on the command line and know you've gotten them
globally.
Ideally, those properties could be computed at the top level without
actually having to remember to call the macro to get them defined, but I'm
not sure that that's worth the effort.
/common.ant.xml
- On lines 232 and 236 you refer to explicitly to "${gwt.svnrev}", but
because you've generalized the property names, this should
be "[EMAIL PROTECTED]". In fact, I think this bolsters the above argument to
back out of overgeneralizing, since it led to you getting it wrong here.
/dev/core/build.xml
- You can actually clean this up quite a bit. The crazy source filtering is
no longer needed at all; it was needed before to have a temporary copy of a
.java file which was an input to javac. Since we're not modifying Java
source anyway, this need goes away. Refactor/rename the task on line 60 to
copy+filter the property file directly into javac.out, and remove line 109.
- Lines 65-66 & 85-86 is what can be merged into a single sentinel file.
/dev/common.ant.xml
- Why do you need to exclude About.properties from both source directories,
since it will only exist in dev/core?
/distro-source/common.ant.xml
- Again, 24-25 & 40-41 can be merged to a single sentinel file.
- 21: What's with the echo here?
On Fri, Sep 5, 2008 at 10:49 PM, Freeland Abbott <[EMAIL PROTECTED]
> wrote:
> We test for whether the sentinel is present, but we do two separate
> filtration passes (first in dev/core for About.{java,properties}, and the
> second in distro-source for the about.{txt,html}). If we had only one file,
> the first pass would touch it, and the second would see it as "available,"
> might see the contents as up-to-date (e.g. if the source about.{txt,html}
> were untouched, destination output newer than that, but still containing the
> "old" SVN info)... net effect, only the first filtration would reliably
> happen without a clean start.
>
>
>
> On Fri, Sep 5, 2008 at 7:14 PM, Scott Blum <[EMAIL PROTECTED]> wrote:
>
>> I haven't fully reviewed the changes, particularly to /common.ant.xml; but
>> I am curious why we need two different sentinel files rather than one file
>> with a composite name?
>>
>>
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
