Bugzilla – Full Text Bug Listing
| Summary: | Move Persist-Annotation to core.di | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] e4 | Reporter: | Thomas Schindl <tom.schindl> | ||||||
| Component: | UI | Assignee: | Project Inbox <e4.ui-inbox> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | bokowski, emoffatt, gunnar, ob1.eclipse, pwebster, remy.suen | ||||||
| Version: | 0.9 | ||||||||
| Target Milestone: | 1.0 RC0 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Mac OS X - Carbon (unsup.) | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
The to be created @Focus-Annotation (bug 310199) should go to the same bundle/package (In reply to comment #0) > I think all annotations should stay together in one place (In reply to comment #1) > The to be created @Focus-Annotation (bug 310199) should go to the same > bundle/package Generally speaking, no, thats not how I see it. DI annotations are an open set. Only annotations useful to everybody (UI, headless, devices, etc) should go into DI packages. (In reply to comment #2) > Generally speaking, no, thats not how I see it. DI annotations are an open set. > Only annotations useful to everybody (UI, headless, devices, etc) should go > into DI packages. Isn't Persist useful to everyone? (In reply to comment #3) > Isn't Persist useful to everyone? I don't know. It was added to serve workbench parts. Unless somebody wants to define a set of generally useful annotations (ha :-)), how about we put annotations into DI packages when we actually need them in more than one place? (And if that is the case for @Persist, then I'll be happy to move it.) Well if you read my full comment it says "I'm fine also if we decide to create a pure UI-Annotation bundle." we also have Execute/CanExecute in why do they go there but Persit doesn't? If you don't want to have them in core.di.annotations then this is not honored by marking this bug as INVALID but we should discuss whether we want a extra bundle for UI-DI annotations. It is ridiculous that I need to add a dependency on ui.workbench itself only because I want to use the Persist (and yet probably to come Focus) annotation. My problem is that in my forward compat layer (singlesourcing for 3.x and 4.0 the otherway round) I now would have to create a dependency on ui.workbench only to provide people the possibility to use Persit (and they themselves have to depend on it to get access to this annotation) This sounds like a scoping issue to me (what is the scope within which using a particular annotation makes sense?)... Truly generic annotations like @Inject/PostConstruct/PreDestroy (Execute?) are obvious candidates for use anyplace DI is desired and should for sure be declared in a core DI bundle. It seems to me that annotations like @Focus, @Persist (@CanExecute?) are tied to the model's rendering implementation. If we want to declare that the implementation *must* use these annotations (+1 for me) then perhaps they should reside in the same bundle as the model's definition (org.eclipse.e4.ui.model.workbench). If we think that the choice of annotations should be up to the folks implementing the model then they should be defined at the implementation level (i.e. in org.eclipse.e4.ui.workbench.renderers.swt). BTW, I'm hoping that the actual annotation is really called 'Persist' not 'Persit'...;-). (In reply to comment #6) > It seems to me that annotations like @Focus, @Persist (@CanExecute?) are tied > to the model's rendering implementation. I believe that these annotations are useful for more than one rendering implementation. If it is in ui.workbench, ui.workbench.swt, ... all components people are writing and use @Persist will create a dependency on it (in fact they create a dependency on nearly the complete e4-code base) which contratics our lightweight programming model. I think it is good practice to hold annotations and service interfaces in a specialized bundle so that people only have to depend on this very small bundle and not our complete codebase! (In reply to comment #8) > I think it is good practice to hold annotations and service interfaces in a > specialized bundle so that people only have to depend on this very small bundle > and not our complete codebase! I agree. I was making the same point to Eric yesterday but then forgot to add it as a comment on this bug. One other point of reference is that "@CanExecute" is used by commands ("org.eclipse.e4.core.commands") and that bundle doesn't depend on UI.
So, to summarize what seems to be the leading sentiment:
- we create a new bundle ("org.eclipse.e4.ui.model.annotations"?);
- we put
@Persist
@UIEventTopic
@Focus [if it gets added]
in that bundle;
- we keep "@CanExecute" in the DI bundle.
Would that work for everybody?
(Alternatively, I like Eric's suggestion to make a new package for annotations in our model bundle.)
I'd name the bundle "org.eclipse.e4.ui.annotations" or "org.eclipse.e4.ui.workbench.annotations" (In reply to comment #10) > - we create a new bundle ("org.eclipse.e4.ui.model.annotations"?); "org.eclipse.e4.ui.di"? > (Alternatively, I like Eric's suggestion to make a new package for annotations
> in our model bundle.)
When moving to the model-bundle we still have a quite huge dependency overhead on EMF and some other e4-core parts.
I think the bundle should have 0 dependencies.
(In reply to comment #12) > (In reply to comment #10) > > - we create a new bundle ("org.eclipse.e4.ui.model.annotations"?); > > "org.eclipse.e4.ui.di"? +1 (In reply to comment #13) > I think the bundle should have 0 dependencies. It will have the following dependencies: Import-Package: javax.inject Require-Bundle: org.eclipse.e4.core.di;bundle-version="0.9.0", org.eclipse.e4.core.di.extensions;bundle-version="0.9.0", org.eclipse.osgi.services;bundle-version="3.2.100", org.eclipse.swt;bundle-version="3.6.0" Created attachment 171010 [details]
Patch
I created and shared in CVS Head bundle "org.eclipse.e4.ui.di"; this patch updates rest of the code.
Changes are applied to CVS Head; I also updated ui.psf file. Paul, could you include the new bundle in the builds? *** Bug 315173 has been marked as a duplicate of this bug. *** (In reply to comment #17) > > Paul, could you include the new bundle in the builds? It's in the I20100603-1940 build. PW (In reply to comment #19) > It's in the I20100603-1940 build. Thank you! |
Created attachment 170723 [details] patch I think all annotations should stay together in one place - This is also interesting from the single-sourcing for 3.x/4.0 store else one needs to depdend on ui.workbench. I'm fine also if we decide to create a pure UI-Annotation bundle.