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

Various fixes #1

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Various fixes #1

wants to merge 11 commits into from

Conversation

OhMyMndy
Copy link

@OhMyMndy OhMyMndy commented Feb 2, 2021

  • Do not continue when Docker build fails
  • Added --docker-opts so options can be passed to docker run
  • Added Shellcheck Github action
  • Remove trailing comma's in devcontainer.json
  • Example devcontainer.json which has comments and trailing comma's in it to test with
  • Replace variables in devcontainer.json (localWorkspaceFolderBasename and localWorkspaceFolder for now)
  • Run chmod + chown when starting, just like Vs Code does

Added github actions for shellcheck
Made devcontainer.sh comply to shellcheck standards
Added logic to devcontainer.sh to change user id and group id
Added logic to replace the variables in devcontainer.json
Added long option for docker run OPTIONS
…soft.com/vscode/devcontainers/base:alpine-3.12" image has it owned by 1000?
devcontainer.sh Outdated
then \
sudo=''
if [[ "$(stat -f -c '%u' $(which sudo))" = '0' ]]; then
Copy link
Author

Choose a reason for hiding this comment

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

For some reason the mcr.microsoft.com/vscode/devcontainers/base:alpine-3.12 image has sudo owned by 1000 and therefore it cannot be used as root


# shellcheck disable=SC2086
docker run -it $DOCKER_OPTS $PORTS $ENVS $MOUNT -w "$WORK_DIR" "$DOCKER_TAG" "$SHELL" -c "\
Copy link
Author

Choose a reason for hiding this comment

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

I run it like this: .devcontainer.sh -v --docker-opts="-u root"

@matkoniecz
Copy link

ping @BorisWilhelms - is there a chance of review (merging/rejecting/commenting)?

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