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

Bug 372002

Summary: API version errors with pie chart color patch
Product: [Tools] MAT Reporter: Andrew Johnson <andrew_johnson>
Component: GUIAssignee: Krum Tsvetkov <krum.tsvetkov>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: kevin.grigorenko
Version: unspecified   
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
Proposed solution with new interface ColoredSlice none

Description Andrew Johnson CLA 2012-02-19 17:26:11 EST 
This gives some API errors in my workspace.
Description	Resource	Path	Location	Type
Missing @since tag on addSlice(int, Color)	PieFactory.java	/org.eclipse.mat.api/src/org/eclipse/mat/snapshot/query	line 133	@since tag problem
Missing @since tag on addSlice(int, String, long, long, Color)	PieFactory.java	/org.eclipse.mat.api/src/org/eclipse/mat/snapshot/query	line 202	@since tag problem
Missing @since tag on addSlice(IObject, Color)	PieFactory.java	/org.eclipse.mat.api/src/org/eclipse/mat/snapshot/query	line 163	@since tag problem

Description	Resource	Path	Location	Type
Missing @since tag on getColor()	IResultPie.java	/org.eclipse.mat.report/src/org/eclipse/mat/query	line 39	@since tag problem
The method org.eclipse.mat.query.IResultPie.Slice.getColor() in an interface that is intended to be implemented has been added	IResultPie.java	/org.eclipse.mat.report/src/org/eclipse/mat/query	line 39	Compatibility Problem

The tricky one is the getColor on IResultPie.Slice
API version errors with pie chart color patch

If a user has extended MAT by implementing IResultPie.Slice then the code won't compile with V1.2
Either we would have to document the API breakage (would this force a major version number increment!) or find another way.
http://wiki.eclipse.org/Evolving_Java-based_APIs_2#Evolving_API_Interfaces
We could invent a new interface extending Slice, or one just providing getColor.

We also need to explain in http://wiki.eclipse.org/index.php?title=MemoryAnalyzer/Contributor_Reference how to set up API compatibility checking using Eclipse API tooling.
Comment 1 Krum Tsvetkov CLA 2012-02-20 10:14:31 EST 
Good that you spotted this. I recently moved to a newer IDE and forgot to setup the API checks, so I didn't notice the problem.

I'll think a bit more what would be the best way to solve this.

>> We also need to explain in http://wiki.eclipse.org/index.php?title=MemoryAnalyzer/Contributor_Reference how to set up API compatibility checking using Eclipse API tooling.
I'll take care to add a description on the page.
Comment 2 Krum Tsvetkov CLA 2012-03-01 10:29:31 EST 
I added some description to the contributor reference:
http://wiki.eclipse.org/MemoryAnalyzer/Contributor_Reference#Configure_API_Tooling_Baseline

With recent IDEs 3.6, 3.7 and 4.2 the tooling is already available in the Eclipse Classic installation. I'm not sure about older IDE versions. I haven't described where to get the API tooling from (in case it isnt present). Do you think we have to add this also? I think it will only make the page longer and more difficult to read.
Comment 3 Andrew Johnson CLA 2012-03-01 11:00:33 EST 
One way of fixing this is to :
define IResultPie.ColoredSlice extends Slice with the getColor() method
in PieFactory, define ColoredSliceImpl to extend SliceImpl and implement getColor()

API question:
Should a ColoredSlice always have a non-null colour? It makes it slightly easier for the consumer and slightly harder for the generator of slices.
If for example we had shaded slides

PieFactory then creates ColoredSliceImpl or SliceImpl depending on whether a color is supplied.
The extra methods are tagged with 1.2

ChartBuilder then checks if it receives a ColoredSlice, and gets the color.
Comment 4 Krum Tsvetkov CLA 2012-03-01 11:08:57 EST 
Created attachment 211894 [details]
Proposed solution with new interface ColoredSlice

I also preferred the solution with a new interface ColoredSlice.
I attached a patch so we could discuss further if needed. But with a few changes we solve the API problem, and it or less exactly what you proposed.
This still assumes color could be null.

If there are no objections, I will check it in tomorrow.
Comment 5 Kevin Grigorenko CLA 2012-03-07 10:56:30 EST 
Hey Guys, so this is just for my education, but why does adding a method cause such problems?

Also, I see the errors report that @since is missing - would adding @since be sufficient to resolve this problem?
Comment 6 Krum Tsvetkov CLA 2012-03-07 10:59:32 EST 
If there are classes which implement the interface which we exposed as an API, then this classes will not compile any longer against the new version of the interface.
I think this is the reason. Since is enough for things which are newly introduced, but which will not break other's coding.
Comment 7 Kevin Grigorenko CLA 2012-03-07 11:13:39 EST 
Ahh, right. I apologize for creating some trouble!
Comment 8 Krum Tsvetkov CLA 2012-03-08 02:14:15 EST 
Do not worry. Thanks for your contributions.
Comment 9 Krum Tsvetkov CLA 2012-07-05 10:39:14 EDT 
this is fixed already.