Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISSUE-651: implemented support for --pid option in DockerCreateContai… #652

Merged
merged 3 commits into from Sep 4, 2018

Conversation

marcoebbinghaus
Copy link
Contributor

adds support for --pid option in docker create / DockerCreateContainer-Task

closes #651

@@ -231,6 +235,10 @@ class DockerCreateContainer extends AbstractDockerRemoteApiTask {
restartPolicy = "${name}:${maximumRetryCount}"
}

void pid(String name) {
pid = "${name}"
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally don't use methods unless there is a good reason. What's the thinking here?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this method. Groovy automatically generates a setter method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good to know. Yes I will remove that method then. Should I push it as a 2nd commit or should I commit locally, squash both commits and force push it into the pr/651 branch so that it remains one commit/push on pr/651?

And I don't get why the Travis CI build failed.
It says "Caused by: org.gradle.api.GradleException: The whole test suite should be executed. The test com.bmuschko.gradle.docker.DockerSpringBootApplicationPluginFunctionalTest.Can create image for a Spring Boot application and push it to DockerHub [#plugin.identifier plugin] was skipped. Please check the execution condition!" .. do I have any influence on that when pushing?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore the Travis CI failure for now as it's a known issue. I would send in another commit with the fix.

moved test from DockerWorkflowFunctionalTest to DockerWorkflowFunctionalTest
@cdancy cdancy added this to the v3.6.2 milestone Sep 4, 2018
@cdancy cdancy merged commit 69e0fb2 into bmuschko:master Sep 4, 2018
@cdancy
Copy link
Collaborator

cdancy commented Sep 4, 2018

All tests pass locally. Thanks @marcoebbinghaus ! Did you want a new patch release done for this change?

@marcoebbinghaus
Copy link
Contributor Author

Thank you for merging @cdancy ! It's okay if it will be in 3.6.2, I don't need a patch release.

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

Successfully merging this pull request may close these issues.

support --pid in DockerCreateContainer
3 participants