More comments in-line On Tue, May 25, 2010 at 11:41 AM, Simon Nash <[email protected]> wrote: > 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 + ?
Senior moment. I meant LICENSE not NOTICE. > >> > 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. For some reason, when I scrolled down the LICENSE file I managed to miss the CDDL bit completely. I see it clearly now so retract this comment. > >> 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/. OK I see your point about the pre-configured domain so let's leave them out. > >> | (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 > > Simon -- Apache Tuscany committer: tuscany.apache.org Co-author of a book about Tuscany and SCA: tuscanyinaction.com
