Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove deprecated core.launcher.Main and equinox.launcher.WebStartMain #562

Merged

Conversation

HannesWell
Copy link
Member

@HannesWell HannesWell commented Mar 23, 2024

Both classes have been deprecated over five years ago via https://bugs.eclipse.org/bugs/show_bug.cgi?id=544262.

There have been requests to keep the classes four years ago in https://bugs.eclipse.org/bugs/show_bug.cgi?id=544262#c29, but as far as I can tell there have been no contribution of the requested tests?
@tjwatson should we proceed with the removal or ask again for feedback? If there are still real users out there, they can still maintain the code by themself. Since they are using WebStart in some way they can probably even test it better than Equinox can do, if there are really no tests.

Copy link

github-actions bot commented Mar 23, 2024

Test Results

   28 files  ±0     28 suites  ±0   11m 55s ⏱️ +34s
2 170 tests ±0  2 124 ✅ ±0  46 💤 ±0  0 ❌ ±0 
2 242 runs  ±0  2 196 ✅ ±0  46 💤 ±0  0 ❌ ±0 

Results for commit c08d455. ± Comparison against base commit f417c37.

♻️ This comment has been updated with latest results.

@HannesWell HannesWell force-pushed the remove_deprecated_mains branch 2 times, most recently from 5ad1c17 to f035f2f Compare March 24, 2024 16:49
@tjwatson
Copy link
Contributor

Lets proceed with deleting this.

@HannesWell
Copy link
Member Author

HannesWell commented Mar 25, 2024

Lets proceed with deleting this.

Great. But there is a surprising number of references in code to that launcher in the SDK that have to be migrated first:

The references in PDE look fine since they handle old target-platforms that probably didn't include the 'new' class.

HannesWell added a commit to eclipse-platform/eclipse.platform.releng.aggregator that referenced this pull request Mar 25, 2024
akurtakov pushed a commit to eclipse-jdt/eclipse.jdt.core that referenced this pull request Mar 26, 2024
HannesWell added a commit to eclipse-platform/eclipse.platform that referenced this pull request Mar 26, 2024
@tjwatson
Copy link
Contributor

Great. But there is a surprising number of references in code to that launcher in the SDK that have to be migrated first:

Thanks for finding these. It is good to get them updated to the new class.

@HannesWell HannesWell force-pushed the remove_deprecated_mains branch 2 times, most recently from 7a6f3fb to dc5176d Compare March 27, 2024 15:49
@HannesWell
Copy link
Member Author

Great. But there is a surprising number of references in code to that launcher in the SDK that have to be migrated first:

Thanks for finding these. It is good to get them updated to the new class.

Your welcome and yes, absolutely. I guess they were simply not discovered since the class was only referenced by its name.

Now that we had a new I-build with all the remaining references replaced (where suitable) and the CI builds passed I think we can finally submit this.

@HannesWell HannesWell merged commit cf85dc1 into eclipse-equinox:master Mar 27, 2024
26 of 28 checks passed
@HannesWell HannesWell deleted the remove_deprecated_mains branch March 27, 2024 18:24
@HannesWell
Copy link
Member Author

HannesWell commented Mar 28, 2024

Looks like this removal went fine and didn't blow up the world. The I-build test results look like before:
https://download.eclipse.org/eclipse/downloads/drops4/I20240327-1800/testResults.php

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants