-
-
Notifications
You must be signed in to change notification settings - Fork 19
Update distZip
task to use the locally published artifacts via publishToMavenLocal
#425
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
Update distZip
task to use the locally published artifacts via publishToMavenLocal
#425
Conversation
@sentry review |
} | ||
|
||
tasks.named("distZip").configure { | ||
this.dependsOn("publishToMavenLocal") | ||
this.doLast { | ||
System.setProperty("maven.repo.local", buildPublishDir) |
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.
so this will tell the task where to publish to?
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.
yep, it then publishes into the specified dir
} | ||
} | ||
platforms.forEach { (distName, projectName) -> | ||
val distribution = if (distName == "main") getByName("main") else maybeCreate(distName) |
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 sure if we need an extension like getOrCreate
something like that. Is it possible other sourcesets also exist and we should getByName in some cases?
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 think we can just use maybeCreate
by default, it will look for it and if it doesnt exist it creates it which is basically a getOrCreate
- updated it
it.replace("module.json", "$projectName$target-$version.module") | ||
} | ||
} | ||
// Rename the POM since craft looks for pom-default.xml |
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.
ugh, I guess ideal-world we wouldn't need this, but would have to change craft again 🙈 maybe makes sense to align this between repos and change craft at some point
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.
LGTM, looks like you're on a streak 🎉
* Update Kotlin and Compose version * Missing newline * Update * Update deprecations * Fix K2 compiler issue * Formatting * Fix test * Update * Update * Formatting * Formatting * Fix Java 11 config * Bump test lib versions * Versions * Java config * Update * Enable debug temporarily * Revert * Update * Update * Update * Line end * Formatting * Add toolchain to java block * Update CHANGELOG * Update publication * Update * Update * Update `distZip` task to use the locally published artifacts via `publishToMavenLocal` (#425) * Update * Update * Update * Let CI run for testing * Update * Update * Update * Update * Update * Clean up * Update use maybeCreate always * Update CHANGELOG.md
Note: this branch merges into the Kotlin
2.1.21
bump branch since the kotlin bump doesn't work with the current/old publication anyway💡 Motivation and Context
Instead of processing artifacts ourselves let's just use the locally published artifacts directly which makes it more robust for the future
💚 How did you test it?
CI test
📝 Checklist
You have to check all boxes before merging:
sendDefaultPII
is enabled.🔮 Next steps