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

container: add container image version #3103

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

Beerosagos
Copy link
Collaborator

This will allow to checkout older commits and run make dockerdev without worrying if the container image version changed.

@Beerosagos Beerosagos requested a review from benma December 10, 2024 16:20
@@ -38,13 +35,15 @@ dockerdev () {
fi

# If already running, enter the container.
if $RUNTIME ps | grep -q $container_name; then
if $RUNTIME ps | grep -q $image_name:$CONTAINER_VERSION; then
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems wrong docker ps shows the running containers, which are named with the container name, not the image name. If I have the same image running under a different name, I don't want dockerdev to use that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, added a second filter 👍

echo "No bitbox-wallet docker image found! Maybe you need to run 'make dockerinit'?" >&2
exit 1
fi
local image_name=bitbox-wallet-app
Copy link
Contributor

Choose a reason for hiding this comment

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

should be shiftcrypto/bitbox-wallet-app, shiftcrypto/ is part of the image name.

This will allow to checkout older commits and run `make dockerdev`
without worrying if the container image version changed.
@Beerosagos
Copy link
Collaborator Author

@benma updated, PTAL 🙏

Copy link
Contributor

@benma benma left a comment

Choose a reason for hiding this comment

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

utACK

@Beerosagos Beerosagos merged commit 7356e0d into BitBoxSwiss:master Jan 7, 2025
9 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.

2 participants