bmuschko / gradle-cargo-plugin

Gradle plugin that provides deployment capabilities to local and remote containers via Cargo
Apache License 2.0
258 stars 63 forks source link

Deployable should be declarable without a context #193

Closed yagotlima closed 5 years ago

yagotlima commented 5 years ago

I don't know if there is a reason for the "context" property being mandatory but my builds are working fine without it.

Since it prevents the deployment of artifacts other than war files I think it's better to make it optional.

bmuschko commented 5 years ago

Thanks for your pull request. Can you please also add a test case for this to DeployableIntegrationSpec?

yagotlima commented 5 years ago

Thanks for your pull request. Can you please also add a test case for this to DeployableIntegrationSpec?

Yes I can. I'll work on it.

yagotlima commented 5 years ago

So I was working on a "can deploy an ear file" test case when I realized it's gonna take longer than I thought initially.

I created a new class: DeployableEarIntegrationSpec to run this test case because it doesn't fit well inside DeployableIntegrationSpec

Turns out I can't extend AbstractIntegrationSpec because it runs tomcat and tomcat doesn't support ear files.

Before I proceed I'd like to know what is expected from this test case?

  1. A "can deploy an ear file" on a new spec class that will need to install Wildfly or some other application server locally;

or

  1. A "can deploy wars without context property" that will verify just that the parameter is indeed optional.
bmuschko commented 5 years ago

The use case we are changing here is "Can deploy a deployable (WAR or EAR or something else) without the need to specify a context". So it's really irrelevant what the deployable is. You can just go with a WAR as part of the test case. At the moment Gradle would complain about it either way if the context isn't provided.

yagotlima commented 5 years ago

Sorry for taking so long. Pushed a new test case just now.

bmuschko commented 5 years ago

Thanks for your work. It has been merged.