SpongePowered / SpongeGradle

Handy gradle utilities for the various gradle projects of SpongePowered
MIT License
21 stars 16 forks source link

Allow setting an artifact instead of setting an configuration #18

Closed BrainStone closed 4 years ago

BrainStone commented 6 years ago

The oreDeploy configuration now allows something like:

oreDeploy {
    artifact = shadowJar
}

This will cause the shadowJar artifact (plus signature) to be uploaded.

Closes #17

dualspiral commented 6 years ago

I'm not a Gradle expert, but the point of using a configuration rather than an artifact is that it keeps all of the related outputs together. All this does is start to make assumptions, while I understand this might look like a preferred approach if your buildscript outputs multiple files, I don't see this as the right thing to do.

I realise that the Nucleus buildscript is a bit messy, but that's partially because I'm not really a Gradle or Groovy user. I could probably do things in a much cleaner way. Honestly, the only real problem is that the task depends on the signArchives task when I need it to depend on signShadow instead. Pretty much everything else is standard, even though I may refer to them as hacks in my buildscript, it isn't really a hack, it's past me not really understanding how things work - the commit was 8 months ago and I haven't revisited that part since because it's not really a hack.

The only thing that might be considered a hack in my buildscript is that I set the changelog at runtime, it would be nice to be able to use a closure on the property instead but it's a minor thing and I might be the only one that really wants that. I may PR that in due course.

Back on topic, setting the sign plugin to sign the shadow configuration then setting the deploy configuration as I'm doing is probably the right way of doing it. This is the real problem which this doesn't solve, this requires you to either sign the archives configuration when you don't want to or stub out the task, the latter of which is what I did. If there is a way to depend on all signing tasks, that would be better, then we just set the configuation as required (ensuring it's signed).

BrainStone commented 6 years ago

There are two issues I have with the configuration solution:

That’s why I opted to use the direct approach instead.

And I think your suggestion to depend on all sign tasks is not bad. Though it may cause the task to depend on unnecessary (and potentially unwanted) tasks.

So I’m not sure whether it’s a good idea.

BrainStone commented 5 years ago

@dualspiral what do you think about that?

gabizou commented 4 years ago

So, considering OreDeploy, it's the last bit of the plugin that's going to be re-evaluated. I believe @Katrix was right to start rewriting it, and now with 0.11 being the newer version that we'll be using, slowly we'll be encouraging migration to use it, once it's been perfected. 0.10.0-SNAPSHOT will remain as is, and no longer developed once 0.11 is released (it's rewritten a few of the plugin usages, so it's important to read the new README).