Summary: | [Model Decoration] Problems with current support for model element decoration | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] Platform | Reporter: | Michael Valenta <Michael.Valenta> | ||||||
Component: | Team | Assignee: | Michael Valenta <Michael.Valenta> | ||||||
Status: | RESOLVED FIXED | QA Contact: | |||||||
Severity: | normal | ||||||||
Priority: | P1 | CC: | bokowski, john.arthorne, Mike_Wilson | ||||||
Version: | 3.1 | Keywords: | api | ||||||
Target Milestone: | 3.2 M6 | ||||||||
Hardware: | PC | ||||||||
OS: | Windows XP | ||||||||
Whiteboard: | |||||||||
Attachments: |
|
Description
Michael Valenta
2006-03-11 18:48:14 EST
Created attachment 36419 [details]
Patch with proposed API changes
Here's the patch with the proposed changs. It includes the new API, cjhanges to CVS to decorate using the new API and changes to JDT/UI to support working set label updates.
Meta-comment: it seems like a lot of new API to be adding without any chance of getting feedback before it's set in stone... ITeamStateDescription getProperties(): getPropertyNames() would make more sense. I would expect getProperties() to return values rather than keys. I guess this is to match the API of IDecorationContext? If so, should this interface extends IDecorationContext? SubscriberTeamStateProvider - There are references in the comments to DecoratedStateProvider (rather than ITeamStateProvider) - should the protected methods and dispose() be final? If not, can subclasses override or extend (must they call super dispose, for example)? TeamStateChangeEvent - Can clients subclass it? Typically events like this aren't subclassed because listeners won't know what subclass they are getting TeamStateProvider - More doc references to decorated state provider - Can clients subclass? SynchronizationStateTester - I don't understand how this class is intended to be used - I'll have to ask you about it - This has all kinds of references to decorated state provider instead of team state provider. - Can it be subclassed? Presumably so. Should it be abstract? - It's odd that clients can directly instantiate this, since it's stateless (all instances will be the same). Created attachment 36493 [details]
Updated path
Here's the updated patch with the changes suggested by Boris and John. In response to the concern of the size of the API, I agree that it is more than usual for this late in the cycle but we didn't get any feedback from clients until post-M5. Althought the API change is not as small as I would like, I am comfortable with the API and have structured it is such a way to solve the issues that were identified by clients and to allow as much flexibility as possible in the future. Also, as a proof of the API, I have also implemented a client (working sets in the packages explorer update properly to team state changes) and a provider (CVS decorates logical model elements with the dirty state and the tag).
Do I have the go-ahead to add this API?
+1. ok to proceed. Thanks. Fix released to HEAD. |