Bug 482382 - API Removal Process is incomplete for removal of types
Summary: API Removal Process is incomplete for removal of types
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: PMC (show other bugs)
Version: 4.6   Edit
Hardware: All All
: P3 major (vote)
Target Milestone: 4.8   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 436505 457134 475833 495330
  Show dependency tree
 
Reported: 2015-11-17 10:36 EST by Markus Keller CLA
Modified: 2017-11-22 07:57 EST (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2015-11-17 10:36:05 EST
The process at https://wiki.eclipse.org/Eclipse/API_Central/API_Removal_Process is incomplete for the removal of types.

Types can be referenced in APIs of dependent bundles, and this causes ripples that need to be considered in the process.

Before a type can be removed, all references must be removed first. If a reference is in another API (supertype, type argument, or type in a method or field signature), then that other API must be removed *before* the type can be removed:

- removal announcements must ripple down the dependency chain
- wait 2 years
- removals must ripple up the dependency chain

Example: Bug 475833 still wants to delete TableTree* types. This is not possible unless all references are removed. But JFace still has APIs with such references that are not even marked as deprecated yet, e.g. org.eclipse.jface.viewers.StructuredViewer.ColorAndFontCollector#applyFontsAndColors(TableTreeItem).

Suggested text for the wiki page:

# Annotate all APIs that are to be removed with @noreference, @noextend and @noimplement where applicable, update deprecation comment and add an entry in the porting guide that lists the fully-qualified name of each API. The deprecation comment and porting guide entry must explain how to adapt the client code and also include a link to the bug report to allow feedback from API adopters.
#* Wording for the Javadoc tags: <code>@noreference This API will be deleted soon, see <a href="https://bugs.eclipse.org/434575">bug 434575</a>.</code>
# Announce the upcoming deletion on all relevant mailing lists, including a link to the bug for community feedback (at least eclipse.org-committers@eclipse.org, cross-project-issues-dev and eclipse-dev). Remind adopters in the announcement to enable API Tools and tell them which API Baseline to use.
# For API types you plan to remove, file a bug for each project in the [[Simultaneous Release]] that uses the types in an API. Removing an API type causes ripples for clients that refer to the type in one of their APIs. Each such client must plan for and announce the removal of the API type <em>at the same time</em>, and must remove its references <em>before</em> the type declaration can be removed.
Comment 1 Lars Vogel CLA 2015-11-17 13:16:29 EST
I think the proposed text is unrealistic. We cannot check every project in the sim release. 

Of course dependent projects like JFace must also react if SWT removes something but that should not stop SWT to remove API. We have seen with the comp 2.0 layer that some project will never react unless the API is actually removed. So if TableTree finally gets removed we can also coordinate the removal of the dependent API in JFace.
Comment 2 Markus Keller CLA 2015-11-17 15:00:13 EST
(In reply to Lars Vogel from comment #1)
> I think the proposed text is unrealistic.

I agree it's a lot of work. But if someone wants to delete a type, these are the unavoidable consequences. I would also agree with a policy that would not allow to delete any type without declaring a breaking change (bump major version).
Comment 3 Lars Vogel CLA 2015-11-17 15:55:03 EST
(In reply to Markus Keller from comment #2)
> (In reply to Lars Vogel from comment #1)
> > I think the proposed text is unrealistic.
> 
> I agree it's a lot of work. But if someone wants to delete a type, these are
> the unavoidable consequences. I would also agree with a policy that would
> not allow to delete any type without declaring a breaking change (bump major
> version).

I think it is not a good approach to define a policy that effectively prevents any deletion of outdated or broken API. That was not the decision of the PMC.
Comment 4 Markus Keller CLA 2015-11-20 12:49:35 EST
(In reply to Lars Vogel from comment #1)
> So if TableTree finally gets removed we can also coordinate the
> removal of the dependent API in JFace.

No, we can't. That's the point of this bug. Please try to understand the problem. It will bite you the next time you try to delete TableTree.

(In reply to Lars Vogel from comment #3)
Wishful thinking is not going to help. The policy makes people believe they are on good track regarding the deletion of an API, but when the 2 years are over, there are only 2 solutions:
- delete the dependent APIs without notice
   => breaks this policy
- announce removal of the dependent APIs and wait another 2 years
   => breaks this policy (have to wait too long)

This policy is incomplete, and you should be the first one interested in making it actually work.
Comment 5 Alexander Kurtakov CLA 2016-06-01 04:26:48 EDT
I can see the problem with JFace. But going through every project in Sim Release and waiting for them to apply patches is clearly unrealistic. 
I am for amending the policy and having everything marked as deprecated and orchestrate it proper but ***only in the scope of Eclipse top level project***.
Comment 6 Lars Vogel CLA 2016-06-01 04:34:58 EDT
(In reply to Alexander Kurtakov from comment #5)
> but ***only in the scope of Eclipse top project***.

+1
Comment 7 Markus Keller CLA 2016-06-01 08:52:29 EDT
> but ***only in the scope of Eclipse top project***.

That would mean API stability is only a concern w.r.t. clients in the Eclipse top-level project. A very narrow view for a project that claims to build a reusable framework. Would you also ditch the rest of the API guidelines just because they are inconvenient?
Comment 8 Lars Vogel CLA 2016-06-01 09:04:25 EDT
(In reply to Markus Keller from comment #7)
> That would mean API stability is only a concern w.r.t. clients in the
> Eclipse top-level project. 

No, the PMC can and will still consider other projects in its API approval process.
Comment 9 Alexander Kurtakov CLA 2016-06-01 09:28:43 EDT
(In reply to Markus Keller from comment #7)
> > but ***only in the scope of Eclipse top project***.
> 
> That would mean API stability is only a concern w.r.t. clients in the
> Eclipse top-level project. A very narrow view for a project that claims to
> build a reusable framework. Would you also ditch the rest of the API
> guidelines just because they are inconvenient?

No, this is putting reasonable boundaries for the guidelines. None of the other projects in the release train are forced to adhere to these guidelines so limiting us in this way makes no sense. 
If we don't want to have narrow view why limit to the sim release? Include JBoss tools, Spring tools, Gnu ARM Eclipse and every other bundle built on top of Eclipse platform - some of these are way more used than many other plugins on the sim.release. You see how it becomes impossible to apply the guidelines at that scope, right?
Finally, no one said that anything about ditching API guidelines for inconvenience. But it also doesn't mean we should continue shipping entirely broken components, years after they have been deprecated, no one cares to fix them for these years for the sake of some plugin not wanting to move even a bit to adhere to our recommendation to move away of what was deprecated years ago.
Comment 10 Alexander Kurtakov CLA 2017-05-05 12:55:00 EDT
It is unclear to me what is expected to happen here. Dani, would you please clarify what you have in mind?
Comment 11 Dani Megert CLA 2017-05-05 13:33:02 EDT
(In reply to Alexander Kurtakov from comment #10)
> It is unclear to me what is expected to happen here. Dani, would you please
> clarify what you have in mind?

Why me?
Comment 12 Alexander Kurtakov CLA 2017-07-26 09:50:17 EDT
As the proposed change is not feasible (opening bug for every possible usage of a type - even in release train). I'm marking this bug as wontfix as no other working solution was proposed. Please reopen with suggestion that the PMC has power over if you have such.
Comment 13 Dani Megert CLA 2017-11-17 11:54:10 EST
The least thing we should request is that all APIs that depend on the deprecated (to be removed) type are also marked as such.
Comment 14 Lars Vogel CLA 2017-11-20 06:55:57 EST
(In reply to Dani Megert from comment #13)
> The least thing we should request is that all APIs that depend on the
> deprecated (to be removed) type are also marked as such.

I added the second sentence:
----
Annotate all APIs that are to be removed with @noreference, @noextend and @noimplement where applicable, update deprecation comment and add an entry in the porting guide. Do the same for API that depends on it.
-----

I think we already were doing this in the past, so this is just a clarification of the process.
Comment 15 Dani Megert CLA 2017-11-22 05:52:30 EST
/org.eclipse.platform.doc.isv/porting/removals.html needs to be updated.
Comment 16 Lars Vogel CLA 2017-11-22 05:55:43 EST
(In reply to Dani Megert from comment #15)
> /org.eclipse.platform.doc.isv/porting/removals.html needs to be updated.

I don't think this is related to this bug. I suggest opening a new bug or adding comments to the existing bugs for the individual entries.
Comment 17 Dani Megert CLA 2017-11-22 06:01:54 EST
(In reply to Lars Vogel from comment #16)
> (In reply to Dani Megert from comment #15)
> > /org.eclipse.platform.doc.isv/porting/removals.html needs to be updated.
> 
> I don't think this is related to this bug.

It is. Please take a look at the document. It refers to this bug.
Comment 18 Alexander Kurtakov CLA 2017-11-22 06:05:34 EST
(In reply to Dani Megert from comment #17)
> (In reply to Lars Vogel from comment #16)
> > (In reply to Dani Megert from comment #15)
> > > /org.eclipse.platform.doc.isv/porting/removals.html needs to be updated.
> > 
> > I don't think this is related to this bug.
> 
> It is. Please take a look at the document. It refers to this bug.

Lars, Dani speaks of the "Note that the policy for removing API types is broken, so some of these plans are not implementable, see bug 482382." line in removals page. It took me some time to realize it as it was not clear from his comment.
Comment 19 Lars Vogel CLA 2017-11-22 06:07:21 EST
(In reply to Dani Megert from comment #17)
> It is. Please take a look at the document. It refers to this bug.

Ah!!!! I see it now.  Shall I update it or are you planning to do that?
Comment 20 Dani Megert CLA 2017-11-22 06:12:28 EST
(In reply to Lars Vogel from comment #19)
> (In reply to Dani Megert from comment #17)
> > It is. Please take a look at the document. It refers to this bug.
> 
> Ah!!!! I see it now.  Shall I update it 

Yes, please.
Comment 21 Eclipse Genie CLA 2017-11-22 06:18:45 EST
New Gerrit change created: https://git.eclipse.org/r/112057