search

antlr / antlr5

BSD 3-Clause "New" or "Revised" License
53 stars 4 forks source link

Migrate v4 to v5 #12

Closed ericvergnaud closed 8 months ago

ericvergnaud commented 8 months ago

Manually merged commits dropped during kotlin runtime integration Disabled testing except for tool and java and kotlin runtimes

ericvergnaud commented 8 months ago

@KvanTTT Gh won't let me do a rebase-and-merge, so I'll have to do a squash-and-merge

ericvergnaud commented 8 months ago

Partially fixes #3

KvanTTT commented 8 months ago

Thanks, I'll review a bit later: today's evening or tomorrow (I also have to work on my official work 😊)

KvanTTT commented 8 months ago

I've encountered the following error during clean project building from your branch:

 Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project antlr5-runtime: Fatal error compiling: error: release version 17 not supported

Have you encountered it?

ericvergnaud commented 8 months ago

Well the reason for the -Xskip-prerelease-check is that without it I can't run the Kotlin tests locally, so it does relate to this PR (although not its title) Removing runtimes is a separate PR not yet filed, in this one I just disabled the CI for them because I can't be bothered fixing it for targets that will go away in the next PR. TBH the value you can bring here is to double-check the Kotlin related changes. But if you absolutely want to spit the PR then you could indeed put the Java version in a preliminary PR.

ericvergnaud commented 8 months ago

Re local build, is your JAVA_HOME pointing to java 17+ ? Or maybe it relates to your local maven settings ?

KvanTTT commented 8 months ago

Re local build, is your JAVA_HOME pointing to java 17+ ? Or maybe it relates to your local maven settings ?

Yes, thanks, it was invalid JAVA_HOME.

KvanTTT commented 8 months ago

Well the reason for the -Xskip-prerelease-check is that without it I can't run the Kotlin tests locally, so it does relate to this PR (although not its title)

Could you clarify which error did you have? I sucessfully run all tests locally without this flag.

ericvergnaud commented 8 months ago

Well the reason for the -Xskip-prerelease-check is that without it I can't run the Kotlin tests locally, so it does relate to this PR (although not its title)

Could you clarify which error did you have? I sucessfully run all tests locally without this flag.

error: class 'org.antlr.v5.kotlinruntime.CharStream' is compiled by a pre-release version of Kotlin and cannot be loaded by this version of the compiler (CharStream is just an example, hundreds of those)

KvanTTT commented 8 months ago

Thanks for the squashing and clear commit message.