Travel Spring added

[TejasM]

No description provided.

[sgilda]

@TejasM : This README.md file has the same formatting issues as the greeter-spring/README.md file. Please open any of the other README.md files in your favorite editor to see how to do the sections.

[TejasM]

Fixed in update, I can squash the commits if you like.

[pmuir]

@joshuawilson could you do a technical review on this?

[sgilda]

@TejasM :

This looks good! The README explains where to look for code differences and is easy to understand.

Under Access the Application, there is one minor issue with the access url. It is missing the -spring and should be: The application will be running at the following URL: http://localhost:8080/jboss-as-travel-spring.

@pmuir : Are we including instructions for Tomcat or should the following line be removed?

NOTE: To deploy the project in EWS-Tomcat5/EWS-Tomcat6, you can use -Pews-tomcat5 or -Pews-tomcat6

[TejasM]

@sgilda: I fixed it locally. I will wait for @pmuir's response before pushing.

[sgilda]

I forgot to mention, when I ran the QS Tools checker, it showed a number of issues with BOM versions.

It also reports problems with illegal characters in the travel-spring/src/main/resources/import.sql file that must be replaced by the unicode value. It looks like the following 2 lines have the problem:

insert into Hotel (id, price, name, address, city, state, zip, country) values (17, 130, 'Hotel Beaulac', ' Esplanade L�opold-Robert 2', 'Neuchatel', '', '2000', 'Switzerland')

insert into Hotel (id, price, name, address, city, state, zip, country) values (22, 250, 'Meli� White House', 'Albany Street', 'Regents Park London', '', 'NW13UP', 'Great Britain')

[sgilda]

@TejasM : You can definitely remove the Tomcat instructions. :-)

[joshuawilson]

@pmuir I can do the tech review.

[joshuawilson]

@TejasM : I am not sure how the license text works on OSS, so can someone else please comment on that? I noticed that most of this app is taken from a Springsoucre Framework example and that is fine. However you added the license text to all the files. Some have no changes, some have only formatting changes, and some have code changes. I thought that under most OSS you needed to submit the changes back to the original source, did you do that? Do you need to?

[sgilda]

@pmuir: Do you know the answer to @joshuawilson's question about the licence text for files coming from the Springsource Framework examples and whether the code chnages have to be submitted back to the source?
Does this also impact the petclinic-spring quickstart in Pull 510?

[LightGuard]

You don't have to submit changed back, it's certainly a good idea though. If the files contained license headers, those MUST stay in the file as they were originally. If not, and we're dealing with a permissive license, we should be fine mentioning in the README were we pulled some code. 
—
Sent from Mailbox for iPhone

On Tue, Jun 18, 2013 at 8:44 AM, sgilda notifications@github.com wrote:

@pmuir: Do you know the answer to @joshuawilson's question about the licence text for files coming from the Springsource Framework examples and whether the code chnages have to be submitted back to the source?

Does this also impact the petclinic-spring quickstart in Pull 510?

Reply to this email directly or view it on GitHub:
#485 (comment)

[sgilda]

A discussion with @TejasM , @pmuir , @joshuawilson on IRC resolved this as follows:

• Existing Spring source files should retain the original license header.
• New source files added to the project that are submitted back to the Spring code base probably should use the Spring license header.
• New source files added specific to the JBoss example that are not submitted back to the Spring code base should use the JBoss license header.

[sgilda]

@TejasM : Do the licenses comply as specified above? Is this ready to review?

[TejasM]

@sgilda: No they licenses haven't been changed to comply as of yet, mostly because travel-spring still needs to be rebased to match the current version from springsource. May be this PR should be closed for now and reopened once that is done?

[sgilda]

That's up to you. It's fine to leave it open in the meantime.

[sgilda]

@joshuawilson and @TejasM plan to bring this back in sync with the original Spring example and only make the changes necessary to get it to run on JBoss. I will hold off merging this one.

[sgilda]

@TejasM reported on 8/2/2013 that he started rebasing this to the original, but got pulled away for more urgent matters. He hopes to get back to it next week.

[sgilda]

Waiting to see about licensing issues.

[sgilda]

@TejasM : I was brought to my attention in JDF-452 that this quickstart needs to target WFK, not EAP.

[TejasM]

@sgilda: thanks for fixing the others and this should now be fixed as well

[TejasM]

Refactored the package names as well as moved it from travel-spring to booking-mvc-spring, to reflect SpringSource

[sgilda]

@pmuir, @TejasM : Can we merge it as it stands now?

[sgilda]

Very nice! Some of my comments above may be wrong. The quickstart runs as "Spring Travel", so maybe some of the comment above are not valid. For example, you can probably ignore my "Booking MVC Spring showcases..." comment.

When I run QS Tools against this, I get a lot of errors.

• There are a few tab space errors.
• There are 28 BOM errors. Some of the BOM errors are for Spring specific libraries, but I think others may be able to use valid BOMs, like commons-logging, some of the h2 libraries, etc.
• There are > 50 file header or license errors. I'm still not clear how we're dealing with the license issues.

[joshuawilson]

@TejasM Can you please add the line back in that you took out and just comment it out. With a comment similar to the README?

I'm referring to: "The only modification needed to get the Spring BookingMVC to work is the removal of <property name="saveOutputToFlashScopeOnRedirect" value="true"/> from FlowHandlerAdapter in webmvc-config.xml. "

[joshuawilson]

@TejasM Why did you need to add the jdbc schema namespace to webapp/WEB-INF/config/data-access-config.xml?

And why did you change the name of the database in the same file?

If there is a valid reason please comment the code and the README.

[joshuawilson]

@TejasM Can you also please add a comment to the README explaining why and what you changed in the POM?

[joshuawilson]

@TejasM Please create a DIFFERENCE.md file to be able to find what you changed in a glance.

[TejasM]

@joshuawilson, @sgilda: Updated, hopefully I got everything.

[sgilda]

What do you think about making this a bulleted list? For example:

• Removed <property name="saveOutputToFlashScopeOnRedirect" value="true"/> from FlowHandlerAdapter in webmvc-config.xml. This is because the property was recently added in a M1 release and not yet in released in a Final Version.
• The pom.xml was changed to leverage the power of JDF-BOMs, in particular the following: jboss-javaee-6.0-with-hibernate, jboss-javaee-6.0-with-spring, and jboss-javaee-6.0-with-tools.

[sgilda]

Back-ticks and typos fixed in the bullted version above.

[sgilda]

Other than the above typos, I do have a couple of questions

1. The QS tools checker still reports a lot of BomVersionChecker and FileHeaderChecker violations, but I'm not sure we can do anything about them. There are also a couple of tab characters.
2. I'm still not clear about licensing issues for this one.

[sgilda]

Comment from @joshuawilson on IRC last evening:

    I just talked with Tejas and he will not be able to get Booking or Grails fixed before the re-org

[sgilda]

Closing for now.