FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add signal query argument for Restart Container command by dmmax · Pull Request #2186 · docker-java/docker-java · GitHub

  1. docker-java
  2. docker-java

Repository navigation

Add signal query argument for Restart Container command - #2186

Merged
eddumelendez merged 7 commits into
docker-java:mainfrom
dmmax:add_signal_to_restart_container_cmd
Oct 27, 2023
ConversationCommits (7)ChecksFiles changed
Merged

Add signal query argument for Restart Container command#2186
eddumelendez merged 7 commits into
docker-java:mainfrom
dmmax:add_signal_to_restart_container_cmd

Conversation

dmmax commented Aug 21, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Linked to the issue: #2123

dmmax requested a review from a team as a code owner August 21, 2023 05:11

eddumelendez commented Aug 21, 2023

Copy link
Copy Markdown
Member

Can you add a test, please?

eddumelendez added this to the next milestone Aug 21, 2023

dmmax commented Aug 21, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Hi @eddumelendez

I added an integration test that covers the new functionality.

I also had to add new API versions to the RemoteApiVersion. Let me know if it's ok for you

BTW The test com.github.dockerjava.cmd.ListContainersCmdIT.testStatusFilter is failing for me as well (on the main branch) on the local machine and only increasing the timeout for stopping the container helps. Is it expected behavior?

This was referenced Aug 24, 2023
Merged
Closed

dmmax commented Sep 4, 2023

Copy link
Copy Markdown
Contributor Author

Hi @eddumelendez
Can you have a look again into the PR? 🙏

eddumelendez reviewed Oct 26, 2023
View reviewed changes
Comment thread docker-java/src/test/java/com/github/dockerjava/cmd/RestartContainerCmdImplIT.java Outdated
.withRegistryUrl("https://index.docker.io/v1/")
.build();
try (DockerClient dockerClient = createDockerClient(dockerClientConfig)) {
if (!getVersion(dockerClient).isGreaterOrEqual(RemoteApiVersion.VERSION_1_42)) {

eddumelendez Oct 26, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think we can use Assume instead.

eddumelendez merged commit fd5da16 into docker-java:main Oct 27, 2023

eddumelendez commented Oct 27, 2023

Copy link
Copy Markdown
Member

Thanks for your contribution, @dmmax !

dmmax deleted the add_signal_to_restart_container_cmd branch October 29, 2023 16:30
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

eddumelendez eddumelendez approved these changes

Assignees

No one assigned

Labels

type/enhancement

Projects

None yet

Milestone

3.3.4

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL