Hi!
> I've checked the artifacts and have a few minor comments. The missing
> version data in the pom is worth fixing I think. The others are not
> particularly serious.
>
> Oh, and how can one easily check what JVM the class format is compatible
> with (ie verify that -target 1.4 was used)?
>
Hmmm ... good question. In linux you can do "file .....class" and it
will print you the version used.
And yes, it was missing in the core pom, I've added it now.
> ===== pom
> In the myfaces-core-1.0.pom, the following dependencies do not specify a
> version:
> * myfaces-api
> * myfaces-shared-orchestra
>
These two are set by the orchestra/maven pom, I thought these versions
will be inherited, aren't they?
> * dependency-maven-plugin
> * build-helper-maven-plugin
> * rat-maven-plugin
> * jxr-maven-plugin
> * surefire-report-maven-plugin
> * maven-taglib-plugin
> * maven-javadoc-plugin
> * maven-source-plugin
>
I am not sure if it is worth it, but in terms of reproducing the build
I've added them now.
> ===== core
>
> The MANIFEST.MF in core.jar seems a little short:
>
> I can't find the docs about what exactly should go here, but I think the
> following should be there at least:
> * orchestra version
> * jvm target version (1.4)
>
I've added
orchestra.artifact = ${pom.artifactId}
orchestra.artifact.version = ${pom.version}
build.target = ${build.target}
The build.target is simply a property used to generate the manifest
entry and used with the compiler plugin.
> ===== core
>
> Minor question: is the name "spring-orchestra-init.xml" unique enough?
> Because we specify this as a resource, bad things will happen if there
> are more than one. An alternative would be
> "org.apache.myfaces.orchestra.spring-init.xml" or something similar.
>
You could also ask if faces-config.xml is unique enough ;-)
I'd prefer to stick with a similar short configuration name, not only to
not introduce another incompatible change ;-)
> ==== javadoc
>
> Currently, package-private methods are being documented. Do we want
> this? It's good for developers but bad for users.
>
I've added show=protected, hope this will make it.
Ciao,
Mario
