firstdarkdev / modpublisher

A dual publishing Gradle Plugin to publish mods to Modrinth, Curseforge and GitHub in one go
MIT License
17 stars 3 forks source link

Idiomatic setters/getters #13

Open MattSturgeon opened 2 months ago

MattSturgeon commented 2 months ago

Kotlin and Groovy both have some built-in coersion of getters and setters into plain object properties.

In Groovy, a getters and setters form what we call a "property", and offers a shortcut notation for accessing and setting such properties. _~Groovy Style guide: Getters and Setters_

Methods that follow the Java conventions for getters and setters (no-argument methods with names starting with get and single-argument methods with names starting with set) are represented as properties in Kotlin ~Calling Java from Kotlin

I believe kotlin's implementation may be incompatible with lombok's default is prefix for boolean methods.

If this is the only issue, it may be fixed by setting lombok.getter.noIsPrefix = true in a lombok.config file.

You can create lombok.config files in any directory and put configuration directives in it. These apply to all source files in this directory and all child directories. ~Configure lombok features


This would need some experimentation, I've noticed when playing around in kotlin codebases that some classes get synthetic properties while others don't.

Examples

E.g. `GHRepository.getFullName()` works: ![image](https://github.com/firstdarkdev/modpublisher/assets/5046562/2c483cb4-28fb-4690-8770-f089cc2d8588) So does `GitHub`: ![image](https://github.com/firstdarkdev/modpublisher/assets/5046562/6f19b345-7e06-4aa0-8bff-dc043928d4c7) While some stuff on `File` works: ![image](https://github.com/firstdarkdev/modpublisher/assets/5046562/2034f622-3f89-4156-a868-b45024089853) And other stuff just doesn't: ![image](https://github.com/firstdarkdev/modpublisher/assets/5046562/023bbcac-2970-4160-a6af-1dd24a6225f8) Maybe this is because `setReadOnly()` isn't actually setting a field, instead it's probably doing filesystem IO.


I believe this would be best as a breaking change, since it'd be a pain to introduce idiomatic setters while still maintaining the existing API. That said, it should be possible and may not be too bad if you think it's worth it...

hypherionmc commented 2 months ago

Wow, this is very insightful.

I agree that this is definitely worth checking out and experimenting with, but might be best kept for a Major release, instead of minor (so maybe 3.0.0).

I have been wanting to expand to other upload platforms, such as [Hangar](https://hangar.papermc.io] from papermc, and our own NightBloom snapshots platform.

I also want to redo the entire CurseForge side, since their API has gotten some new stuff that isn't properly supported, and just smacked in because it was needed, and then the modrinth ID caching and all that good stuff.