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

Refactor buildscript to Kotlin DSL #518

Closed

Conversation

alexstaeding
Copy link
Contributor

@alexstaeding alexstaeding commented Jun 8, 2021

I am opening this preliminary migration from Groovy to Kotlin DSL as a draft at first because it still needs to be properly tested.

The jar appears to build and run successfully but I am not able to test publishing myself (mavenLocal works though). I have made several assumptions about how the scripts should be organized, and am open to discussing any sensible changes.

There are some elements still missing, such as the license and developer information in VelocityPublishPlugin.kt. (Example here)

@kashike
Copy link
Member

kashike commented Jun 8, 2021

I'd recommend looking into applying indra first - see wiki for details.

@kashike kashike added the type: feature New feature or request label Jun 8, 2021
@astei
Copy link
Contributor

astei commented Jun 9, 2021

I would recommend applying as much of indra as you can - it will likely eliminate all of the custom Gradle plugins you currently employ (with the exception of licensing - indra uses a single license header AFAIK, we use multiple).

@@ -45,7 +45,6 @@
*
* @param invocation the invocation context
* @return the tab complete suggestions
* @implSpec defaults to wrapping the value returned by {@link #suggest(CommandInvocation)}
Copy link
Contributor

Choose a reason for hiding this comment

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

I recall this gave you a compile error, but this is a problem with the build tool, not the javadoc. See https://openjdk.java.net/jeps/8068562 and https://stackoverflow.com/a/55150289 for details. If we really need to remove it, move the contents to a new paragraph.

@BomBardyGamer
Copy link

BomBardyGamer commented Nov 4, 2021

Can I recommend the new version catalogues feature introduced in Gradle 7.0 over declaring versions in gradle.properties? You can declare all of your dependencies in a TOML file and then Gradle will generate type safe accessors, such as libs.guava, that you can use in code. It really helps centralise dependency declarations, and can make it easier if you need to use a dependency in more than one module. More info here.

Yes, it's still a preview, but some big projects like ViaVersion have started using them, so I reckon they're safe.

@BomBardyGamer
Copy link

BomBardyGamer commented Nov 4, 2021

Also, I recommend that you use separate convention plugins in a separate build-logic folder that you can include in the build for configurations and adding plugins to subprojects, as it helps separate build logic from the scripts, and helps you reuse things between sub projects. For reference projects to view for this, I can say Adventure and ViaVersion do this well.

@alexstaeding
Copy link
Contributor Author

Can I recommend the new version catalogues feature introduced in Gradle 7.0 over declaring versions in gradle.properties? You can declare all of your dependencies in a TOML file and then Gradle will generate type safe accessors, such as libs.guava, that you can use in code. It really helps centralise dependency declarations, and can make it easier if you need to use a dependency in more than one module. More info here.

Yes, it's still a preview, but some big projects like ViaVersion have started using them, so I reckon they're safe.

@BomBardyGamer thanks for pointing that out, I just looked at it briefly and it looks promising.

Also, I recommend that you use separate convention plugins in a separate build-logic folder that you can include in the build for configurations and adding plugins to subprojects, as it helps separate build logic from the scripts, and helps you reuse things between sub projects. For reference projects to view for this, I can say Adventure and ViaVersion do this well.

Is that not the point of the buildSrc folder? I'm not sure where you could really draw the line between a "plugin local to the project" and "build logic" so I don't really see a big advantage to separating them.

As for the very old age of this PR, I'm aware that this is needed soon and am getting back to work on it. I got stuck for a while on the indra conversion due to what appears to be a bug that causes the signing plugin not to recognize my secret keyring (only when indra is used, my normal script works).

I'm probably just going to leave indra out for now, and probably open an issue about the signing bug.

For the sake of finishing this quickly, I don't see the use of indra so important as to further delay the merging of this PR.
(Unless, of course, someone has the magic indra code that works with the signing plugin)

@alexstaeding alexstaeding marked this pull request as ready for review November 22, 2021 10:46
@alexstaeding alexstaeding marked this pull request as draft November 22, 2021 10:55
@kashike
Copy link
Member

kashike commented Dec 9, 2021

@alexstaeding Are you able to update this pull request again? I pushed a few changes that break this a bit.

@alexstaeding
Copy link
Contributor Author

@alexstaeding Are you able to update this pull request again? I pushed a few changes that break this a bit.

Yeah, I'll have a look at it in the next few days.

astei added a commit that referenced this pull request Jan 1, 2023
Spiritually indebted to #518 and @alexstaeding.

There's a minor break - we're going up to 3.2.0-SNAPSHOT as the API now compiles against Java 11. But this is more academic in practice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants