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

Bump Eclipse version and require Maven 3.9.0 #149

Merged
merged 1 commit into from
Jul 16, 2024
Merged

Conversation

ctubbsii
Copy link
Member

No description provided.

@ctubbsii ctubbsii added this to the 3.5.0 milestone Jul 16, 2024
@ctubbsii ctubbsii requested a review from hazendaz July 16, 2024 21:58
@ctubbsii
Copy link
Member Author

I tried to fix more things by adding dependency convergence, but I couldn't figure out where the dependency versions were coming from. I can hard-code in a bunch of them to fix their versions and force dependency resolution, but it would be fragile if dependencies changed in the future. As is, though, a lot of the complexity needed in the formatter-maven-plugin to get dependency convergence is really coming from this library, because it doesn't have dependency convergence of its own.

Related question: is upstream Eclipse ever going to publish jsdt-core themselves? It'd be nice to stop packaging a custom version for use with formatter-maven-plugin.

@hazendaz
Copy link
Member

This module is for javascript. Its smaller than it was but doesn't solve issue here #50.

Not to be confused with the java code one. AFAIK, eclipse still does not produce this for central. This is using the OSGI p2 repository to pull the artifacts since they are not in maven. Eventually would like to solve the issue above so we have a good balance on what this repo needs vs other users using it. If we were to try modular on this project, this is a current show stopper as a lot of overlap.

The dependency convergence issue if I'm not mistaken was the java one as Eclipse was shipping that with version ranges which are now fixed since 2022. This one doesn't have any version ranges listed to it.

It is a bit of foo magic. I don't care for OSGI and barely understand its purpose. It took a lot of testing adjusting that file until our builds worked well. I tried adding some back per the open ticket but had not gotten back to it recently. I don't think fixing that is a rush. It is much smaller in general so that helps improve formatter build time when loading the m2 cache.

@@ -60,6 +60,7 @@ SPDX-License-Identifier: EPL-2.0
<mdep.analyze.skip>true</mdep.analyze.skip>
<!-- rat plugin is no good at validating EPL headers out of the box -->
<rat.skip>true</rat.skip>
<revelc.min-build-mvn>3.9.0</revelc.min-build-mvn>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just require 3.9.8? 3.9.0 is pretty buggy, so was the first few. 3.9.8 is best stable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just the minimum. We can recommend newer. This minimum was derived automatically from the plugins configured in the pom.xml... they require at least 3.9.0.

@hazendaz
Copy link
Member

LGTM other than comment on why not maven 3.9.8. Feel free to merge away :)

@ctubbsii ctubbsii merged commit 0dc9cb6 into revelc:main Jul 16, 2024
6 checks passed
@ctubbsii ctubbsii deleted the 2024-06 branch July 16, 2024 23:31
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.

2 participants