Thanks for these comments.  See responses below.

  Simon

Simon Laws wrote:
,
On Sat, May 22, 2010 at 12:03 AM, Simon Nash <[email protected]> wrote:
Please review and vote on RC1 of the SCA Travel Sample 1.0 release.

The distribution artifacts and RAT report are available for review at:

http://people.apache.org/~nash/tuscany/travelsample-1.0-RC1/

The release tag is at:

https://svn.apache.org/repos/asf/tuscany/sca-java-1.x/tags/travelsample-1.0-RC1/

There are no maven artifacts included in this release.

As this has been posted on a weekend, I'll wait 72 hours from start of
business Monday (US Pacific Time) before closing the vote.

 Simon



I downloaded the RC, looked at the files, built with ant and ran all
the samples from the launcher directory. Looks really good for a first
RC. A few comments....


README
  note1 - would be good to include specific version of Tuscany this
version of the sample has been tested against (1.6)

Thanks, good catch.  I'll also add a comment to say that people shouldn't
try to build or run this using 2.x.

 Typo 2/3s way down top level
    Yuscany - > Tuscany

Will fix.

NOTICE
  should include LICENSE for JAXWS artifacs. CDDL + ?
>
I don't see anything in CDDL that requires me to add this to the NOTICE file.
The CDDL licensing terms are covered in the LICENSE file.  My understanding
of the recently clarified ASF guidelines for NOTICE files is that we don't
need to duplicate the contents of the LICENSE file here.

  are all the openejb dependencies ASL2 licensed?

Yes, all those that are packaged in the travel sample are ASL2.

Samples
  All ran OK from ant. I had a few minor comments on the contents of
table 1. Excerpts follow:

| 3) Introducing       | introducing-client          | ant run-domain
|                                       |OK but need DM URL
http://localhost:9990/ui/home/
|    (distributed)     |                             | ant run-trips
|                                       |OK
|                      |                             | ant run-tours
|                                       |OK
|                      |                             | ant run
|                                       |OK

| 8) Full application  | fullapp-nodes               | ant run-domain
|                                       |OK but need DM URL
http://localhost:9990/ui/home/
>
I didn't include the domain manager URLs for these two because they run
from predefined configurations and therefore there's no need for the
user to browse to these URLs to run the scenario.  If we add them, we'd
need to explain in the preceding set of notes that they are FYI and
don't need any user input.

Admittedly I haven't been 100% consistent on this because I did list a
?wsdl URL for one of the webapps which is not required to run the app.
So I'm certainly open to adding these with the appropriate disclaimer.

If we add these, I'd prefer to use http://localhost:9990/ui/workspace/
rather than the blank screen produced by http://localhost:9990/ui/home/.

|    (distributed)     |                             | ant run
|                                       |OK but need SCATours URL
http://localhost:8080/scatours

Omitted in error.  I'll fix it.

| 10) Reference        | payment-java-reference-pass | ant run
|                                       |OK
|     passing          |                             | ant run
|                                       |Why repeated?

It's a cut and paste error.  I'll fix it.

  Simon

Reply via email to