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

Random Vogon fixes #12

Closed
wants to merge 9 commits into from
Closed

Random Vogon fixes #12

wants to merge 9 commits into from

Conversation

vanam
Copy link
Contributor

@vanam vanam commented Jul 12, 2024

Hi,
firstly, I would like to thank you for this amazing demo.

There are several minor improvements which might make your demo one bit more amazing.

So Long, and Thanks for All the Fish

@vanam
Copy link
Contributor Author

vanam commented Jul 12, 2024

Strange thing happens when I run ./gradlew resolveAndLockAll --write-locks. Every time it messes up some org.jetbrains.kotlin dependencies.

It turns

org.jetbrains.kotlin:kotlin-stdlib-common:2.0.0=compileClasspath,productionRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath
org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.9.24=compileClasspath,productionRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath
org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.24=compileClasspath,productionRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath

to

org.jetbrains.kotlin:kotlin-stdlib-common:2.0.0=compileClasspath,testCompileClasspath
org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.9.24=compileClasspath,testCompileClasspath
org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.24=compileClasspath,testCompileClasspath

and then ./gradlew build fails:

Execution failed for task ':tea-service:cyclonedxBom'.
> Could not resolve all dependencies for configuration ':tea-service:runtimeClasspath'.
   > Resolved 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.24' which is not part of the dependency lock state
   > Resolved 'org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.9.24' which is not part of the dependency lock state
   > Resolved 'org.jetbrains.kotlin:kotlin-stdlib-common:2.0.0' which is not part of the dependency lock state

I have to manualy remove this change from changelist. Is it also happening to you?

Copy link
Owner

@jonatan-ivanov jonatan-ivanov left a comment

Choose a reason for hiding this comment

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

Thank you for the PR, I'm glad you like the project.
This PR does a lot of things, some I don't want to merge (right now/as-is) and some I would like to merge in but not all in one PR but separately.

If you don't want to open separate PRs (it's work), please let me know and I will cherry pick your commits and merge those separately.

.github/dependabot.yml Show resolved Hide resolved
.github/workflows/gradle.yml Show resolved Hide resolved
build.gradle Show resolved Hide resolved
core/gradle.lockfile Show resolved Hide resolved
docker-compose.yml Show resolved Hide resolved
eureka/src/main/resources/logback-access.xml Show resolved Hide resolved
gradle/wrapper/gradle-wrapper.properties Show resolved Hide resolved
tealeaf-service/tealeaf-api/build.gradle Show resolved Hide resolved
@jonatan-ivanov
Copy link
Owner

Strange thing happens when I run ./gradlew resolveAndLockAll --write-locks. Every time it messes up some org.jetbrains.kotlin dependencies.
[...]
I have to manualy remove this change from changelist. Is it also happening to you?

Unfortunately it does, I think this is caused by okhttp that uses the Kotlin SDK, it's on my list to fix it, I created an issue to track it: #13
I'm not a fan of having the Kotlin SDK on the classpath anyways so this can mean replacing okhttp (maybe OpenFeign too) for the new Interface Clients in Spring Framework but I need to check if instrumentation works with the customizations I have and if there is Logbook support available.

@vanam
Copy link
Contributor Author

vanam commented Jul 13, 2024

I guess we can close this PR now.

@jonatan-ivanov
Copy link
Owner

Thank you very much!

@jonatan-ivanov
Copy link
Owner

I think the merged PRs and these commits:

  • e4a6cfa
  • a84d86a
    should cover the changes in this PR, please let me know if this is not the case.

Thank you very much once again for the contributions!

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

Successfully merging this pull request may close these issues.

2 participants