Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

Make JVM target explicit in sentry-core #462

Merged
merged 1 commit into from
Jun 17, 2020

Conversation

dilbernd
Copy link
Contributor

IDEA’s gradle support failed to inherit language/bytecode level from
parent project, causing mockito failures w/ 11 bytecode.

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Explicitly set Java / JVM versions for the sentry-core subproject.

💡 Motivation and Context

Technically it doesn’t fix a Sentry bug but rather papers over a bug in IDEA 2020.1.2 which makes tests fail from the IDE: It doesn’t detect the source & target JVM versions from the parent project. Then it generates JVM 11 bytecode, which our Mockito can’t handle.

💚 How did you test it?

Reimported in IDEA 2020.1.2 and saw that the JVM versions were now correct in the settings & the tests were green.

Had some tests that did not seem to deliver any result (green or red) in AS 4.0 but this did not change after reverting this change locally so it’s at least unrelated.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes - ⚠️ Build config only
  • All tests passing

🔮 Next steps

IDEA’s gradle support failed to inherit language/bytecode level from
parent project, causing mockito failures w/ 11 bytecode.
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

LGTM but this is really up to @marandaneto

@dilbernd
Copy link
Contributor Author

Forgot to note under “how tested”: Local ./gradlew clean test on OpenJDK Runtime Environment (Zulu 8.46.0.19-CA-macosx) (build 1.8.0_252-b14) also ran through.

@marandaneto marandaneto merged commit d1bbcad into getsentry:master Jun 17, 2020
@marandaneto
Copy link
Contributor

ideally, we'd set this on allprojects but I remember having issues with Kotlin DSL, it works just fine using Groovy though, let's bring this in and I'll check if they already fixed the compatibility with Groovy Clojures and Kotlin DSL.
thanks for the fix @dilbernd

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

Successfully merging this pull request may close these issues.

3 participants