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

Add Gradle-like API for adding dependencies #382

Merged
merged 1 commit into from
Aug 25, 2022
Merged

Add Gradle-like API for adding dependencies #382

merged 1 commit into from
Aug 25, 2022

Conversation

ileasile
Copy link
Contributor

Fix #367

@ileasile ileasile added bug Installation and functionality issues feature labels Aug 17, 2022
@@ -20,7 +20,7 @@ abstract class ScriptTemplateWithDisplayHelpers(

fun EXECUTE(code: String) = host.scheduleExecution(CodeExecution(code).toExecutionCallback())

fun USE(library: LibraryDefinition) = host.addLibrary(library)
fun USE(library: LibraryDefinition) = host.scheduleExecution { addLibrary(library) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we try to add library immediately, recursive execution happens - execution DURING another execution. We don't know how to handle it, because we use previous script instances for evaluation, and it's not clear what to do without them. So we just schedule the execution to the end of the current execution.

Copy link
Contributor

@altavir altavir left a comment

Choose a reason for hiding this comment

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

It is not clear if this uses gradle resolver or maven resolver. It works differently for mpp libraries. I think the notation is great in any case, but if it uses maven resolver, there must be a warning about jvm suffix.

Copy link
Contributor

@nikolay-egorov nikolay-egorov left a comment

Choose a reason for hiding this comment

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

Looks neat!

@ileasile
Copy link
Contributor Author

I'm concerned about names too: it's not clear what gradle means, it sounds like we configure Gradle, not kernel. But I can't come up with a better name for this DSL entry point
Maven resolver is used underhood, yes.

@altavir
Copy link
Contributor

altavir commented Aug 18, 2022

I'm concerned about names too: it's not clear what gradle means, it sounds like we configure Gradle, not kernel. But I can't come up with a better name for this DSL entry point Maven resolver is used underhood, yes.

maybe something like requires? It does not matter how it is called since you copy only what is inside.

@ileasile
Copy link
Contributor Author

  1. gradle - clear what you should write inside but doesn't explain the purpose
  2. gradleLike[Conf] - better, but verbose
  3. requires - good borrowing from JS modules, but there is require in Kotlin, and may be misleading
  4. config, configure, provide - not bad, but too generic
  5. build - looks like a compromise variant
  6. dependencyResolutionManagement - name of block in settings.gradle[.kts] that is actually responsible for dependency management

Let's choose something please

@altavir
Copy link
Contributor

altavir commented Aug 20, 2022

@ileasile I think it is better to discuss in slack. Maybe a vote.

I do not think that the outer scope should match how it looks in Gradle because people do not copy the whole block, they copy its contents. Also dependencyResolutionManagement is not used in project config, only in settings.

What about dependsOn{ }?

…ies code) via USE(). Add Gradle-like API for adding dependencies.

Fix #367
@ileasile
Copy link
Contributor Author

Final version of API:

USE {
	repositories {
		mavenCentral()
	}
	dependencies {
		implementation("io.github.config4k:config4k:0.4.2")
	}
}

Or in integration:

internal class Integration : JupyterIntegration() {
    
    override fun Builder.onLoaded() {
        repositories {
		mavenCentral()
	}
	dependencies {
		implementation("io.github.config4k:config4k:0.4.2")
	}
    }
}

@ileasile ileasile merged commit a4df334 into master Aug 25, 2022
@ileasile ileasile deleted the feat_367 branch April 29, 2024 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Installation and functionality issues feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a function to configure dependencies and repositories in gradle-like manner
3 participants