Skip to content

Allow setting an artifact instead of setting an configuration#18

Closed
BrainStone wants to merge 1 commit into
SpongePowered:masterfrom
BrainStone:master
Closed

Allow setting an artifact instead of setting an configuration#18
BrainStone wants to merge 1 commit into
SpongePowered:masterfrom
BrainStone:master

Conversation

@BrainStone
Copy link
Copy Markdown

@BrainStone BrainStone commented Sep 27, 2018

The oreDeploy configuration now allows something like:

oreDeploy {
    artifact = shadowJar
}

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

Closes #17

@dualspiral
Copy link
Copy Markdown

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
Copy link
Copy Markdown
Author

There are two issues I have with the configuration solution:

  • Configurations are ambiguous. They are collections of files. Which means the closure used to filter the collection may match several files which results in ambiguous behavior.
  • The build script required to make it work with configurations is overly complicated if all I want to do is define which file to upload.

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
Copy link
Copy Markdown
Author

@dualspiral what do you think about that?

@gabizou gabizou closed this Oct 17, 2019
@gabizou
Copy link
Copy Markdown
Member

gabizou commented Oct 17, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants