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

fix(down): Fix down command if specified services are not running #12164

Merged
merged 2 commits into from
Sep 26, 2024

Conversation

idsulik
Copy link
Collaborator

@idsulik idsulik commented Sep 26, 2024

What I did
Fixed down command, it passes empty services if specified services are not running, in this case we stops all services despite command line argument where we specify list of services we want to stop

Related issue
#12163

(not mandatory) A picture of a cute animal, if possible in relation to what you did
image

Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>
Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>
Copy link
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @idsulik 🙏

@glours glours requested review from a team, ndeloof and jhrotko and removed request for a team September 26, 2024 08:06
Copy link
Contributor

@ndeloof ndeloof left a comment

Choose a reason for hiding this comment

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

We try to avoid mock-based tests vs e2e tests, as those based on mocks are a pain to maintain, while e2e are way more explicit and can be easily ran with plain command line for experiment - (even they are slower to execute)

@idsulik
Copy link
Collaborator Author

idsulik commented Sep 26, 2024

We try to avoid mock-based tests vs e2e tests,

I'll keep it in mind, thank you

@ndeloof ndeloof merged commit 87f457e into docker:main Sep 26, 2024
28 checks passed
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