Community
Participate
Working Groups
Created attachment 251374 [details] Result of accessing the application Default Dynamic Web Project does not contain index.html and so running it on a server always require to generate this file first. That's a very typical scenario and should be fixed for a more seamless experience. Accessing the application at http://localhost:8080/helloworld/ shows "Forbidden" as shown in the attachment forbidden.png. It could be a check box "Generate index.html" see attachment "GenerateIndexHtml.png"
Created attachment 251375 [details] Proposal wizard change
Good enhancement. Targeting initially to WTP 3.7, and adding helpwanted keyword. A patch is welcome :)
Created attachment 251795 [details] Patch
probably it is no need to add index.html generation in AppClientFacetInstallDelegate.createFlexibleProject
Roberto, could you review my patch, please?
(In reply to Daniel Azarov from comment #5) > Roberto, could you review my patch, please? Hi Daniel, Thank you for the patch. I quickly reviewed it, and noticed a couple of issues: 1. The code to create the checkbox for creating index.html was added to J2EEModuleFacetInstallPage. But this change is only for web projects, so I suggest adding it to WebFacetInstallPage instead. 2. Same comment for changes in IJ2EEFacetInstallDataModelProperties and J2EEFacetInstallDataModelProvider, I think IWebFacetInstallDataModelProperties and WebFacetInstallDataModelProvider should be used instead. 3. Not sure why the patch includes changes in Application client module classes, but I do not think these classes should change. Maybe the changes can be avoided by moving the code to the classes I suggest in 1 and 2. Those are the comment I have so far
Created attachment 252148 [details] Patch
Roberto, thank you for all your comments, could you review my new patch?
(In reply to Daniel Azarov from comment #8) > Roberto, thank you for all your comments, > could you review my new patch? Hi, Thank you for the new patch, and sorry for the delay for reviewing it again. I have some more comments after reviewing the latest patch: - Please update copyright header in java files: if the header has only one year (like Copyright (c) 2003 IBM Corporation and others.), add the second year (like Copyright (c) 2003, 2015 IBM Corporation and others.). If it already has 2 years, update the second to be 2015) - In IWebFacetInstallDataModelProperties, the new property is: public static final String GENERATE_INDEX_HTML = "IJ2EEFacetInstallDataModelProperties.GENERATE_INDEX_HTML"; I suggest changing "IJ2EEFacetInstallDataModelProperties.GENERATE_INDEX_HTML" to "IWebFacetInstallDataModelProperties.GENERATE_INDEX_HTML" - In WebFacetInstallDelegate, why not generating an HTML 5 file? Just courious. I'm OK with what you propose. - In JavaEEPreferencesInitializer, the new preference is called APPLICATION_GENERATE_INDEX_HTML. Given that this is related to web projects only, I suggest using something like DYNAMIC_WEB_GENERATE_INDEX_HTML or WEB_GENERATE_INDEX_HTML, because the APPLICATION prefix is more related to EAR projects - In JavaEEPreferencesInitializer, remove the preference APP_CLIENT_GENERATE_INDEX_HTML. I think is not used/needed - In WebFacetInstallPage.properties, the new message should have the & character, to check/uncheck it when the user presses Alt, like the "Generate web.xml deployment descriptor" label. It could be "Generate &index.html"
Created attachment 252909 [details] Patch Roberto, thank you for your comments, could you review my new patch?
(In reply to Daniel Azarov from comment #10) > Created attachment 252909 [details] > Patch > > Roberto, thank you for your comments, > could you review my new patch? Thank you, the patch looks good. I'm asking Chuck's review. Chuck, this includes changes in UI, so if approved, would need PMC approval. Also, the 3.7 M7 build is happening this week (today I believe), so, lets see if this can be included in 3.7 or will need to wait until next release.
Chuck, any movement on the issue?