search

eclipse-equinox / equinox.bundles

Eclipse Public License 2.0
8 stars 16 forks source link

Remove 'o.e.equinox.serverside.sdk' feature (and stop building and publishing it) #43

Closed HannesWell closed 2 years ago

HannesWell commented 2 years ago

As discussed in https://github.com/eclipse-equinox/equinox.bundles/pull/42#issuecomment-1117623651 there is now known use-case for the org.eclipse.equinox.serverside.sdk and because it makes the project maintenance more difficult (due to its cross-repository plug-in inclusions) this feature is proposed for removal.

Please speak up as soon as possible if you use that feature and describe your use-case.

In case nobody raises objections we will proceed with the removal Do we need PMC approval for this? If necessary, @tjwatson, @vogella please discuss this at the next PMC-meeting.

HannesWell commented 2 years ago

Announced the removal at the equinox-dev mailing-list: https://www.eclipse.org/lists/equinox-dev/msg09419.html

mknauer commented 2 years ago

It is currently required by our target definition that we are using to build the Eclipse RAP Tools project

org.eclipse.rap.tools.target.definition.target:

            <unit id="org.eclipse.equinox.serverside.sdk.feature.group" version="3.23.400.v20220428-0821"/>

and will require some work to remove it. Please let us know in advance before merging this PR.

mickaelistria commented 2 years ago

@mknauer Are you actually using this feature or can RAP just stop consuming it? If RAP needs it and seems to be the only consumer, wouldn't it be better that you move this feature into RAP's code?

mknauer commented 2 years ago

Using: yes Stopping consuming it: possible

I was just asking to get an early heads-up before you are merging this PR, not questioning its removal, because it means some work for me.

mickaelistria commented 2 years ago

I was just asking to get an early heads-up before you are merging this PR

I think this is the heads-up; the goal would be to merge this PR as soon as it's ready -it could be later today!-; so the sooner RAP can cope with it the better.

mknauer commented 2 years ago

Done. Dependency of the RAP project has been removed.

mickaelistria commented 2 years ago

Great, thanks @mknauer !

HannesWell commented 2 years ago

Done. Dependency of the RAP project has been removed.

Thank you for that. In general I think, if there are remaining use-cases in almost all is possible to replace the serverside.sdk with the previously included features/Plugins.

mickaelistria commented 2 years ago

@HannesWell Anything left to do here? Would be nice to merge it soon.

HannesWell commented 2 years ago

@HannesWell Anything left to do here? Would be nice to merge it soon.

Technically this is ready for submission. The only question is if this removal requires a special approval (although I expect it will be granted since @vogella and @tjwatson expressed their consent in PR #42)?

mickaelistria commented 2 years ago

if this removal requires a special approval

I think it's all good, with public announce on the mailing list and with 1 Project Lead and 2 PMC member approvals.

HannesWell commented 2 years ago

if this removal requires a special approval

I think it's all good, with public announce on the mailing list and with 1 Project Lead and 2 PMC member approvals.

Alright, then lets merge this. :)

FYI, I won't be available tomorrow, so if this requires more changes somewhere else in the platform to make the I-build succeed I want to ask you to take care of them.

HannesWell commented 2 years ago

Should we mention the removal in the migration-guide or similar? And if so, where exactly do I have to do this?

vogella commented 2 years ago

Should we mention the removal in the migration-guide or similar? And if so, where exactly do I have to do this?

Mentioning should not be necessary IMHO. If you want you can add to the N&N.

mickaelistria commented 2 years ago

Migration guide is at https://github.com/eclipse-platform/eclipse.platform.common/tree/0effdecb3a20c5be80642c874adf318b8199d4b5/bundles/org.eclipse.platform.doc.isv/porting/4.24

HannesWell commented 2 years ago

I was preparing an entry in the migration but started to get the impression that the migration guide is for more heavy weight incompatibilities like major API changes. On the other hand my impression is that N&N mostly contains cool new features to advertise the new release. I think the removal of this feature is not such cool interesting item. 😅

I think we should definitely announce it but I'm not sure where the best place is.

Could you clarify what the exact difference between N&N and migration guide is and what the target-audience is?

vogella commented 2 years ago

The API migration guide for planned API removals. The N&N is to inform users about notable changes. As features not not API, I think it is more interesting for N&N if at all.

tjwatson commented 2 years ago

I find it hard to think of removal of a feature as N&N to any user. That really feels like migration guide material. You say they are not API, but I am sure lots of consumers would disagree if we removed the RCP feature or any other that is heavily depended on.

vogella commented 2 years ago

I have no strong opinion on this...........

tjwatson commented 2 years ago

I have no strong opinion on this...........

Well since you put it that way, I have no strong opinion either :)

But an opinion was asked for.

HannesWell commented 2 years ago

I find it hard to think of removal of a feature as N&N to any user. That really feels like migration guide material. You say they are not API, but I am sure lots of consumers would disagree if we removed the RCP feature or any other that is heavily depended on.

Agree on this. I created an migration guide entry: https://github.com/eclipse-platform/eclipse.platform.common/pull/20 Feel free to add your comments.

I have no strong opinion on this...........

Well since you put it that way, I have no strong opinion either :)

But an opinion was asked for.

Even if opinions are not strong they can still be helpful. Thanks.

HannesWell commented 2 years ago

Agree on this. I created an migration guide entry: eclipse-platform/eclipse.platform.common#20 Feel free to add your comments.

Since there were no objections I submitted it.