Bug 227318 - CommonFrameworksPlugin preferences is broken
Summary: CommonFrameworksPlugin preferences is broken
Status: CLOSED FIXED
Alias: None
Product: WTP Java EE Tools
Classification: WebTools
Component: jst.j2ee (show other bugs)
Version: 3.0   Edit
Hardware: PC Windows XP
: P3 major (vote)
Target Milestone: 3.0 RC1   Edit
Assignee: Konstantin Komissarchik CLA
QA Contact: Chuck Bridgham CLA
URL:
Whiteboard: PMC_approved
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2008-04-16 08:58 EDT by Neeraj Agrawal CLA
Modified: 2008-06-06 13:38 EDT (History)
5 users (show)

See Also:
ccc: pmc_approved?
cbridgha: pmc_approved? (raghunathan.srinivasan)
cbridgha: pmc_approved? (naci.dai)
deboer: pmc_approved+
cbridgha: pmc_approved? (neil.hauge)
cbridgha: pmc_approved? (kaloyan)
konstantin: pmc_approved?
konstantin: review?


Attachments
Proposed patch (8.67 KB, patch)
2008-04-16 13:16 EDT, Neeraj Agrawal CLA
no flags Details | Diff
Updated patch (8.74 KB, patch)
2008-04-18 15:40 EDT, Neeraj Agrawal CLA
bjorn.freeman-benson: iplog+
Details | Diff
Additional patch (for jst.common.project.facet.core) (8.72 KB, patch)
2008-05-14 20:51 EDT, Konstantin Komissarchik CLA
no flags Details | Diff
Additional patch (for jst.j2ee) (1.08 KB, patch)
2008-05-14 20:51 EDT, Konstantin Komissarchik CLA
no flags Details | Diff
Additional patch (for jst.common.project.facet.core) (8.04 KB, patch)
2008-05-14 20:54 EDT, Konstantin Komissarchik CLA
no flags Details | Diff
Additional Patch (10.23 KB, patch)
2008-05-14 20:56 EDT, Konstantin Komissarchik CLA
bjorn.freeman-benson: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Neeraj Agrawal CLA 2008-04-16 08:58:21 EDT
Build ID: 3.4 M6

Steps To Reproduce:
Due to refactoring of CommonFrameworksPlugin, the clients utilizing java source folder preferences are broken
Comment 1 Neeraj Agrawal CLA 2008-04-16 13:16:54 EDT
Created attachment 96286 [details]
Proposed patch
Comment 2 Neeraj Agrawal CLA 2008-04-18 15:40:08 EDT
Created attachment 96648 [details]
Updated patch
Comment 3 Chuck Bridgham CLA 2008-05-06 13:00:51 EDT
I approve
Comment 4 Chuck Bridgham CLA 2008-05-06 17:54:52 EDT
Needed to refactor these prefs..   Currently they are broken.
Comment 5 Carl Anderson CLA 2008-05-12 11:19:49 EDT
Comitted to HEAD with one modification- removed the addition of
Eclipse-LazyStart: true
since the MANIFEST.MF already has
Bundle-ActivationPolicy: lazy
(Eclipse-LazyStart is the deprecated way to do it, the Bundle-ActivationPolicy is its replacement, and they both do the same thing, so no sense having both)
Comment 6 David Williams CLA 2008-05-14 04:14:22 EDT
somehow my approval got removed (Carl :) ... so, I'm re-adding now, just so the summary pages make sense. 
Comment 7 Konstantin Komissarchik CLA 2008-05-14 20:30:42 EDT
Guys,

I hate being a pain about this, but this change introduced new public API during RC1 which really isn't necessary and we certainly don't want to be on the hook to support it.

On a related note, I see that the fact we don't have a "jst.common" bugzilla component created some confusion here (and explains why I am only seeing this bug now). We should probably do something about that. Either we should create a new "jst.common" component or better yet, rename "wst.common" to just "common".
Comment 8 Konstantin Komissarchik CLA 2008-05-14 20:51:21 EDT
Created attachment 100341 [details]
Additional patch (for jst.common.project.facet.core)
Comment 9 Konstantin Komissarchik CLA 2008-05-14 20:51:49 EDT
Created attachment 100342 [details]
Additional patch (for jst.j2ee)
Comment 10 Konstantin Komissarchik CLA 2008-05-14 20:54:15 EDT
Created attachment 100343 [details]
Additional patch (for jst.common.project.facet.core)

Forgot to remove the x-friends declaration.
Comment 11 Konstantin Komissarchik CLA 2008-05-14 20:56:52 EDT
Created attachment 100345 [details]
Additional Patch

Sorry for all the noise. Ignore previous attachments. Here is a consolidated patch.
Comment 12 Konstantin Komissarchik CLA 2008-05-14 21:03:11 EDT
Please review "Additional Patch". This patch moves the introduced FacetCorePlugin class into an internal package and updates the reference from jst.j2ee plugin. This is basically a minimum change that is necessary to hide the introduced api, but more thought will need to go into how these product flags are handled in a future release. It seems like we have at least three different ways of doing the same thing. Maybe it's time to think about retiring some of these.
Comment 13 Tim deBoer CLA 2008-05-15 14:28:09 EDT
Approved.
Comment 14 Konstantin Komissarchik CLA 2008-05-15 15:09:32 EDT
Fix released.
Comment 15 Neeraj Agrawal CLA 2008-06-06 13:38:02 EDT
verified, closing