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

Added command 'down', to bring down containers #2774

Merged
merged 3 commits into from
Apr 4, 2023

Conversation

fwalloe
Copy link
Contributor

@fwalloe fwalloe commented Apr 1, 2023

Added the command 'down', which can be used to bring down the docker containers. Equivalent to running 'docker-compose down'

Added the command 'down', which can be used to bring down the docker containers. Equivalent to running 'docker-compose down'
Copy link
Member

@jaschaurbach jaschaurbach left a comment

Choose a reason for hiding this comment

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

Please add it to the list of commands available (at the bottom of the file)

@mouse-reeve
Copy link
Member

I'm not sure if it's a good idea to replicate docker commands in bw-dev when they don't do anything different than running then from docker-compose? It's clear to me why it's useful to have commands that are longer, involve multiple steps, or have specific flags are beneficial to have in the file, but it feels brittle to me to have bw-dev also encompass regular docker commands.

nb: The one place where this is already the case is build, which I also think perhaps shouldn't be a command in bw-dev

Added 'down' to list of of commands
@fwalloe
Copy link
Contributor Author

fwalloe commented Apr 4, 2023

@jaschaurbach Added it to the list of commands

@mouse-reeve That's a fair point and I would tend to agree, but so long as there's a command to start the container in bw-dev, there should also be an equivalent command to bring it down.

Copy link
Member

@jaschaurbach jaschaurbach left a comment

Choose a reason for hiding this comment

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

Thanks.

@jaschaurbach
Copy link
Member

I am here with @fwalloe - as long. We habe be-dev up and we should have be-dev down.

@mouse-reeve
Copy link
Member

That seems reasonable enough to me, thank you both!

@jaschaurbach jaschaurbach merged commit e909cbf into bookwyrm-social:main Apr 4, 2023
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.

3 participants