-
Notifications
You must be signed in to change notification settings - Fork 3
Configure publishing via settings DSL instead of gradle properties #33
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
base: main
Are you sure you want to change the base?
Conversation
4acf3af to
c682fa6
Compare
autonomousapps
left a comment
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.
Just some minor nits!
| * enabled.set(true) | ||
| * repo { | ||
| * url.set("https://repo.example.com/maven") | ||
| * username.set("myuser") | ||
| * password.set("mypassword") |
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.
Not a big deal, but Kotlin DSL magic lets us use = now instead of set(...).
Although, that said, I usually prefer for my DSLs to only expose functions and not the properties directly. I think it's more maintainable in the end.
| * Sets the repository username and disallows further changes. Convenience method for Groovy | ||
| * DSL. |
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 mean, I would consider just exposing this for both DSLs.
| private fun Project.configureSandbagPom(pom: MavenPom) { | ||
| private fun Project.configureArtifactPom(pom: MavenPom) { |
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 is a better naming convention.
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.
| private lateinit var extension: ArtifactSwapExtension | ||
|
|
||
| override fun apply(target: Settings) = target.run { | ||
| extension = extensions.create("artifactSwap", ArtifactSwapExtension::class.java) |
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 usually like to embed this in the extension class as a factory function.
| gradle.settingsEvaluated { | ||
| logger.lifecycle("publish enabled ${extension.publishing.enabled.get()}") | ||
| if (extension.publishing.enabled.get()) { |
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.
Why is it important to evaluate that property here? (I'm just always suspicious of things that "look like" afterEvaluate. Is this preferable to checking the final value of enabled in each project context?
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.
the values from extensions are never available in the initial plugin application, you can only read them in afterSettings (or afterEvaluate for plugins)
|
|
||
| val Project.sandbagHashFile: File | ||
| get() = File(rootDir, providers.gradleProperty(SANDBAG_HASH_FILE).get()) | ||
| val Project.artifactHashFile: File |
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.
Is there a reason we're using java.io instead of the Gradle API (like RegularFile)?
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.
thats the type that using file() in build.gradle gave me.
c682fa6 to
ae741a5
Compare

No description provided.