Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 345514

Summary: Compilation errors in HEAD
Product: [WebTools] WTP Java EE Tools Reporter: Rob Stryker <stryker>
Component: jst.j2eeAssignee: Rob Stryker <stryker>
Status: RESOLVED FIXED QA Contact: Chuck Bridgham <cbridgha>
Severity: normal    
Priority: P3 CC: ccc, david_williams
Version: unspecifiedFlags: david_williams: pmc_approved+
cbridgha: pmc_approved? (raghunathan.srinivasan)
cbridgha: pmc_approved? (naci.dai)
cbridgha: pmc_approved? (deboer)
cbridgha: pmc_approved? (neil.hauge)
cbridgha: pmc_approved? (kaloyan)
cbridgha: pmc_approved? (cbridgha)
cbridgha: review+
Target Milestone: 3.3 RC1   
Hardware: PC   
OS: Linux   
Whiteboard: PMC_approved
Attachments:
Description Flags
Compilation error patch none

Description Rob Stryker CLA 2011-05-11 23:35:09 EDT
There are several compilation errors in trunk. Summary as follows:

 1) ComponentExportOperation - unnecessary null checks
 2) ArchiveFactoryImpl - unnecessary null checks
 3) EJBJar20VRule - unnecessary null checks
 4) WebServicesNavigatorContentProvider - local instance hiding superclass instance
 5) WebAppProvider - local instance hiding superclass instance
 6) AddListenerWizardPage - local instance hiding superclass instance
 7) JEEDeployableFactory - local instance hiding superclass instance

The solutions in the following patch are as follows:
  1) Remove null check
  2) Remove null check
  3) Remove null check
  4) Rename subclass's "viewer" to "viewer2"
  5) Rename subclass's "children" to "children2", "text" to "text2"
  6) Rename subclass's "sychHelper" to "synchHelper2"
  7) Rename subclass's "moduleDelegates" to "moduleDelegates2", then noticed variable was unused, then deleted variable.
Comment 1 Rob Stryker CLA 2011-05-11 23:38:17 EDT
Created attachment 195455 [details]
Compilation error patch
Comment 2 Rob Stryker CLA 2011-05-11 23:46:58 EDT
Setting up for review, Chuck. Would like to get JEE tools repo in order before a major release. It'd be a big shame if a major release went out with the source code from teh tag not compiling ;) 

Pre-emptively filling out the PMC reviews:

    * Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such. 

   It's unprofessional and unacceptable to have a repository for an upcoming major release which, were a user or adopter to check out from a tag, be non-compilable. These patches fix that. 

    * Is there a work-around? If so, why do you believe the work-around is insufficient? 
 
   The workaround is for the user to check out the jeetools plugins, and change each plugin's compiler settings to be more liberal with errors, or, for a user to manually fix the errors himself. Either way leaves the user with a workspace inconsistant with head. 

    * How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added? 

    The patch has merely been smoketested. All variable renames were done via the eclipse rename refactor mechanism, which, if JDT is to be trusted, changed only those variables which were already using the subclass's version of the variable. 

    * Give a brief technical overview. Who has reviewed this fix? 

   Unnecessary null checks were removed. Fields hiding superclass fields were renamed by adding a "2" at the end of the name. Unused fields were removed. 

    * What is the risk associated with this fix? 

   Risk is very low.
Comment 3 Rob Stryker CLA 2011-05-11 23:56:52 EDT
pushing for rc1
Comment 4 Rob Stryker CLA 2011-05-12 01:13:28 EDT
*** Bug 335073 has been marked as a duplicate of this bug. ***
Comment 5 Chuck Bridgham CLA 2011-05-12 09:46:04 EDT
These changes are good - thanks - adding PMC flag
Comment 6 David Williams CLA 2011-05-12 12:07:55 EDT
I'm sort of on the fence with this one ... but will 'approve'. On the fence since not "real" errors ... warning that have been set to "error". Are those settings set the the project? or workspace? If not set on a project level, they should be. If they are set on a project level, then all the more reason to fix ... though still late. How can such "late" errors be discovered earlier in future?
Comment 7 Carl Anderson CLA 2011-05-12 13:41:06 EDT
(In reply to comment #6)
> I'm sort of on the fence with this one ... but will 'approve'. On the fence
> since not "real" errors ... warning that have been set to "error". Are those
> settings set the the project? or workspace? If not set on a project level, they
> should be. If they are set on a project level, then all the more reason to fix
> ... though still late. How can such "late" errors be discovered earlier in
> future?

They've been discovered for quite a while, by Rob himself - see bug 334948 , which was split off to bug 335073 , which was closed as a duplicate of this.
The settings are on a per-project level, but these are issues that arose because of the compiler changes that were made in base Eclipse, and then further refined in later milestones.

There are two other parts of this - one for Common Tools - bug 345518 - and one for EJB Tools - bug 335075 - that should also come up for approval today.
Comment 8 Carl Anderson CLA 2011-05-12 15:58:20 EDT
Committed to HEAD for WTP 3.3 RC1.

Also for the plugins for which these were the only changes this release, I increased the plugin version ID.