Velocity icon indicating copy to clipboard operation
Velocity copied to clipboard

Refactor buildscript to Kotlin DSL

Open alexstaeding opened this issue 4 years ago • 7 comments

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)

alexstaeding avatar Jun 08 '21 03:06 alexstaeding

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

kashike avatar Jun 08 '21 06:06 kashike

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).

astei avatar Jun 09 '21 02:06 astei

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 avatar Nov 04 '21 08:11 BomBardyGamer

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.

BomBardyGamer avatar Nov 04 '21 09:11 BomBardyGamer

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 avatar Nov 14 '21 12:11 alexstaeding

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

kashike avatar Dec 09 '21 18:12 kashike

@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.

alexstaeding avatar Dec 09 '21 23:12 alexstaeding