On 8/19/06, Craig McClanahan <[EMAIL PROTECTED]> wrote:
Thanks for the detailed review!  Responses inline, in preparation for a
second round of test uploads.  (I also filed issue SHALE-257 to track the
individual commits made in response to these comments.)

<snip/>

Thanks for the fixes and clarifications. Couple of comments below
(everything I really cared about has already been fixed).


* Cruft in following Bundle.properties (such as the
> org.apache.shale.tiles.TilesViewHandler section) should be removed:
>
>
> $shale-core/src/main/resources/org/apache/shale/resources/Bundle.properties


Not addressed this time around ... I don't have any automated tools to
validate what bundle keys are actually used, and don't want to disrupt
things at this point in time.

<snap/>

Agreed, I was relying on a grep in shale-core and the fact that
shale-tiles is its own artifact.



If possible, it would be great if applicable Shale jars' manifests
> also adopt the Jakarta Commons non-standard attributes regarding JVM
> targets (m1 docs):
>
> http://jakarta.apache.org/commons/releases/prepare.html#checkjarmanifest
>
> More of a reassurance, than anything else, for those of us having
> deployments on 1.4.


Maven 2 generated MANIFEST.MF files include a "Build-Jdk" header to document
what was used to actually build this jar, but it's not going to be
particularly reassuring :-).  It says "1.5.0_07" in every case, because that
is the JDK used to create the entire build (so that it would build the
1.5dependent stuff too).  An examination of the POMs, however, will
show you
that the source and target levels default to 1.4, and are only set to 1.5 on
the things that depend on that (shale-tiger, shale-sql-browser,
mailreader-jpa, and shale-mailreader-jpa).

<snip/>

Correct. The reason we adopted the manifest approach (additionally) in
Jakarta Commons was to allow users such as me to be lazier (rather
than go find the POM or project.properties for m1 builds). It appears
that m2 additionally packages these in the jars themselves in
META-INF/maven, so this should no longer be necessary.

-Rahul

Reply via email to