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

Instantiate ClassFileReader via reflection #3090

Merged
merged 1 commit into from
Jun 17, 2024
Merged

Instantiate ClassFileReader via reflection #3090

merged 1 commit into from
Jun 17, 2024

Conversation

cdietrich
Copy link
Member

Fixes #3089

Fixes #3089

Signed-off-by: Christian Dietrich <[email protected]>
@cdietrich cdietrich added this to the Release_2.36 milestone Jun 14, 2024
@cdietrich cdietrich requested a review from szarnekow June 14, 2024 11:53
@LorenzoBettini
Copy link
Contributor

Aren't we going to move to the new jdt anyway? As part to the Java 17 minimum version?

In any case, I'd avoid code duplication and put the code in some utility method.

@cdietrich
Copy link
Member Author

problems

  • new jdt maven wont be published before the Xtext release
  • likely newer platform artifacts will even require java 21 to run.

=> we would have to drop out from 2024-09 or explode there and or set minimal version to 21
i dont know what the exact plans there are though

@cdietrich
Copy link
Member Author

@LorenzoBettini where would the parkplatz for this utility be

@LorenzoBettini
Copy link
Contributor

@LorenzoBettini where would the parkplatz for this utility be

For example, xtext.java?

In general, we might also want to create some kind of JDT Facade to isolate the rest of the code as much as possible from this kind of problems; that's something I've always wanted to propose but always forgot to.

@cdietrich
Copy link
Member Author

but xbase.testing does not depend to does it?

@LorenzoBettini
Copy link
Contributor

I would go with the project that uses JDT internals, which are used by both projects involved in this PR, and move that utility method there; I don't have an Xtext workspace at the moment.

In any case, I can work on that later or in the near future without blocking this PR.

@cdietrich
Copy link
Member Author

lets see what @szarnekow says

@szarnekow
Copy link
Contributor

I would land this here. It’s unfortunate that JDT breaks in such a simple way but there’s not much we can do about that. Subsequently we could perform a systematic refactoring to extract code into a new bundle. That won’t help with old versions though. Same as this PR does only help for future versions

@LorenzoBettini
Copy link
Contributor

@cdietrich so shall we merge this?

@cdietrich cdietrich merged commit 1cb4744 into main Jun 17, 2024
11 checks passed
@cdietrich cdietrich deleted the cd-issue-3089 branch June 17, 2024 06:57
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.

Fix api breakage with newest jdt
3 participants