-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Build: Make bson-kotlin and bson-kotlinx optional #1148
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
Conversation
Reduce the dependency tree for driver-core by making the kotlin dependencies optional. Kotlin-coroutine and Kotlin-sync now take a hard dependency on bson-kotlin. So they will work out the box. Users will now have to opt into bson-kotlinx support. JAVA-5036
@@ -65,6 +65,7 @@ dependencies { | |||
|
|||
api(project(path = ":bson", configuration = "default")) | |||
api(project(path = ":driver-reactive-streams", configuration = "default")) | |||
implementation(project(path = ":bson-kotlin", configuration = "default")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adds a direct dependency - should we also add kotlinx here as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure. What are the pros and cons?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With just bson-kotlin:
Pros: Minimizes the dependencies - doesn't drag in kotlinx serialization.
Cons: bson-kotlinx becomes an extra opt in requirement.
implementation("org.mongodb:mongodb-driver-kotlin-coroutine:4.10.0")
implementation("org.mongodb:bson-kotlinx:4.10.0")
I think this PR is the best course of action - the automatic extra dependency on bson-kotlin is small. So don't think we should add kotlinx automagically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm just not sure how widely kotlinx serialization will be used. I guess we can always make it non-optional in the future. We may need a documentation update for this.
@@ -19,7 +19,6 @@ import org.jetbrains.kotlin.gradle.tasks.KotlinCompile | |||
plugins { | |||
id("org.jetbrains.kotlin.jvm") | |||
`java-library` | |||
kotlin("plugin.serialization") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wasn't used or needed - same goes for bson-kotlinx in the test.
@@ -65,6 +65,7 @@ dependencies { | |||
|
|||
api(project(path = ":bson", configuration = "default")) | |||
api(project(path = ":driver-reactive-streams", configuration = "default")) | |||
implementation(project(path = ":bson-kotlin", configuration = "default")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure. What are the pros and cons?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should document this is a breaking change, even though it's doubtful anyone will be affected by it.
@@ -65,6 +65,7 @@ dependencies { | |||
|
|||
api(project(path = ":bson", configuration = "default")) | |||
api(project(path = ":driver-reactive-streams", configuration = "default")) | |||
implementation(project(path = ":bson-kotlin", configuration = "default")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm just not sure how widely kotlinx serialization will be used. I guess we can always make it non-optional in the future. We may need a documentation update for this.
Reduce the dependency tree for driver-core by making the kotlin dependencies optional. Kotlin-coroutine and Kotlin-sync now take a hard dependency on bson-kotlin. So they will work out the box. Users will now have to opt into bson-kotlinx support. JAVA-5036
Reduce the dependency tree for driver-core by making the kotlin dependencies optional.
Kotlin-coroutine and Kotlin-sync now take a hard dependency on bson-kotlin. So they will work out the box. Users will now have to opt into bson-kotlinx support.
JAVA-5036