Re: [cdt-dev] Fwd: Re: Change 154830 in cdt/org.eclipse.cdt[master]: Bug

[Jonah Graham <jonah@xxxxxxxxxxxxxxxx>]

Hi Sergei,

Unfortunately I cannot approve this change - not on its own at this
time anyway. This change breaks binary compatibility. The effect of
changing the return type is that the method is being removed from the
API and a new method is being added. This would require a major
version bump for the bundle - something that seems overkill for the
purpose of this cleanup. (Note for those new to the conversation - the
change Sergei is requesting is the parts of
https://git.eclipse.org/r/#/c/154830/1/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/CUIPlugin.java
that don't have a +1)

What is needed is a plan to do a new major version of CDT at some
point that can break API. I am happy to have this sooner rather than
later - but once we do it we should try our absolute best not to have
another major version increment for a while (number of years) - so
there needs to be compelling reason to do this. The last major bump
was to remove the CDI debug implementation and that lead to CDT 9.0.

The other option is look to the Eclipse Platform deprecation and
deletion process. The Platform skirts around the API semantic
versioning issue with the policy:
https://wiki.eclipse.org/Eclipse/API_Central/Deprecation_Policy. I
don't object to adopting this policy for CDT, but until now that has
not been the case.

As for the specific use of deprecated types in CDT's API we can start
by deprecating it and marking it for deletion* - whether the deletion
is as a result of a major release or a new policy is to be determined.
In addition all uses (in CDT) of the newly deprecated methods should
be updated too.

* There is no place to mark for deletion in CDT yet AFAIK - we could
have something like
https://help.eclipse.org/2019-09/topic/org.eclipse.platform.doc.isv/porting/removals.html
or we can do it in the New and Noteworthy (as was done for 9.0 -
https://wiki.eclipse.org/CDT/User/NewIn90#API_modifications).

I am sorry that what looks on the surface like a trivial fix has so
much baggage. Please let me know how you would like to proceed and I
will support you in anyway I can.

Thanks,
Jonah

~~~
Jonah Graham
Kichwa Coders
www.kichwacoders.com

On Mon, 23 Dec 2019 at 05:11, <sergei.kovalchuk@xxxxxxxxxx> wrote:
>
> Thanks a lot!
> To proceed the cleanup story in "org.eclipse.cdt.ui" bundle, I would
> like to change the deprecated type for public API
> from  "CUIPlugin: public ContextTypeRegistry
> getCodeTemplateContextRegistry()"
> to    "CUIPlugin: public ContributionContextTypeRegistry
> getCodeTemplateContextRegistry()".
>
> Currently, inside the "cdt" repository we have
> the next sensitive dependency:
>
> 1) org.eclipse.cdt.internal.corext.template.c
>      FileTemplateContext
>      FileTemplateContext(String, String)
> 2) org.eclipse.cdt.internal.ui.preferences
>      CodeTemplateBlock
>       edit(TemplatePersistenceData, boolean)
>       getFileTemplateContextRegistry()
>       updateSourceViewerInput(List<Object>)
> 3)org.eclipse.cdt.ui
>      CUIPlugin
>        getCodeTemplateStore()
>
>
> Could I have "approve" for such changes?
>
>
> Best Regards,
> Sergei.
>
>
>
> On 2019-12-20 15:53, Jonah Graham (Code Review) wrote:
> > Jonah Graham has posted comments on this change. (
> > https://git.eclipse.org/r/154830 )
> >
> > Change subject: Bug 558474 - [CDT]CleanUp deprecated methods
> > ......................................................................
> >
> >
> > Patch Set 6: Code-Review+2
> >
> > LGTM
> >
> > Thank you and well done on your first CDT Gerrit.
> >
> >
> > --
> > To view, visit https://git.eclipse.org/r/154830
> > To unsubscribe, visit https://git.eclipse.org/r/settings
> >
> > Gerrit-Project: cdt/org.eclipse.cdt
> > Gerrit-Branch: master
> > Gerrit-MessageType: comment
> > Gerrit-Change-Id: Iff82715d7415d0512a8a1fe3f9625e7c27fb01d8
> > Gerrit-Change-Number: 154830
> > Gerrit-PatchSet: 6
> > Gerrit-Owner: Sergei Kov <sergei.kovalchuk@xxxxxxxxxx>
> > Gerrit-Reviewer: Alexander Fedorov <alexander.fedorov@xxxxxxxxxx>
> > Gerrit-Reviewer: CDT Bot <cdt-bot@xxxxxxxxxxx>
> > Gerrit-Reviewer: Jonah Graham <jonah@xxxxxxxxxxxxxxxx>
> > Gerrit-Comment-Date: Fri, 20 Dec 2019 12:53:00 +0000
> > Gerrit-HasComments: No
> _______________________________________________
> cdt-dev mailing list
> cdt-dev@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or unsubscribe from this list, visit
> https://www.eclipse.org/mailman/listinfo/cdt-dev