Conversation
|
There are currently some comments in the install.sh file so I can debug CI checks if needed. I will remove them before final approval. |
.github/workflows/test.yaml
Outdated
| # Debian-based | ||
| - debian:latest | ||
| - debian:bullseye | ||
| - ubuntu:latest | ||
| - ubuntu:22.04 | ||
| - ubuntu:20.04 | ||
| # Microsoft dev containers | ||
| - mcr.microsoft.com/devcontainers/base:ubuntu | ||
| - mcr.microsoft.com/devcontainers/base:debian |
There was a problem hiding this comment.
optional: I think testing on just the latest images is probably fine.
src/doppler-cli/install.sh
Outdated
| echo "Checking for curl..." | ||
| if ! type curl >/dev/null 2>&1; then | ||
| echo "Installing curl..." | ||
| apt-get update -y && apt-get -y install --no-install-recommends curl ca-certificates |
There was a problem hiding this comment.
blocking: This all assumes that this is a debian based system with apt-get installed, which is not a valid assumption. Take a look at how some of microsoft's features install from different package managers. We should support redhat variants and alpine, at a minimum (and probably also test those).
There was a problem hiding this comment.
Also, be sure to clean up the package lists after you're done, to keep the user's layer size small.
.github/workflows/release.yaml
Outdated
| push: | ||
| branches: [main] |
There was a problem hiding this comment.
nit: I don't know that we necessarily want this on every push, we likely want it to be manual and only run it when the version changes.
| { | ||
| "source": "doppler-cli-user-config", | ||
| "target": "/doppler", | ||
| "type": "volume" | ||
| } |
There was a problem hiding this comment.
check: Is it safe/valid to have a volume shared between all containers that might use this feature?
There was a problem hiding this comment.
This is a good question and I struggled with where to go for this. The volume gets shared across devcontainers, but the devcontainers stay in 1 location. Meaning if someone has multiple devcontainers running, those containers should be contained to their local machines and not be shared with other engineers. Where this could become an issue is if a team is working on a shared Docker host, but that seems very atypical of devcontainer usage. Having a volume means users don't have to reauthenticate across every dev container if they're running multiple or on every single devcontainer build. I could be talked into either direction on this.
There was a problem hiding this comment.
It's been a long time since I looked at this, but as I recall, the volumes get a persistent, unique ID appended to the name when the container is started, so I don't think this should be a problem unless they're literally using the exact same devcontainer, which seems unlikely. Most ways I've seen this used, each developer has their own devcontainer running.
amoses12
force-pushed
the
austin/cli
branch
2 times, most recently
from
463d473 to
03fc764
Compare
3 participants
This PR adds a few things to the POC that already existsed:
This will require CLI changes to be deployed in order for CI tests to pass: PR 512