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.

Sign up for GitHub

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

Jump to bottom

Docker Images #1235

Merged
merged 5 commits into from
Oct 15, 2017
Merged

Docker Images #1235

merged 5 commits into from
Oct 15, 2017

Conversation

robertohuertasm
Copy link
Member

Just improved the docker images:

  • reduced file size by using node:alpine.
  • fixed issue with vscode.d.ts error when using some linux docker images.
  • added a docker image in order to get the latest artifact from the published version, not the local. This may be useful for cases like vscode-icons for Github extension where the author requires the latest icons.json file.

//cc @dderevjanik

@robertohuertasm robertohuertasm requested a review from a team October 12, 2017 20:41
@codecov
Copy link

codecov bot commented Oct 12, 2017 •
edited

Codecov Report

Merging #1235 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1235   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          47      47           
  Lines        2550    2540   -10     
  Branches      128     127    -1     
======================================
- Hits         2550    2540   -10
Impacted Files Coverage Δ
test/settings/settings.test.ts 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0c1c3e...afce9b0. Read the comment docs.

JimiC
JimiC previously approved these changes Oct 13, 2017
View reviewed changes
@JimiC
Copy link
Member

JimiC commented Oct 13, 2017

I don't know much about docker (I know that this might come as a surprise to you) so I guess my approval is just typical.

jens1o
jens1o reviewed Oct 13, 2017
View reviewed changes
Copy link
Member

@jens1o jens1o left a comment

Choose a reason for hiding this comment

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

I don't know anything about Docker in detail, sorry.

@dderevjanik
Copy link
Contributor

Hi,
thank you very much for providing remote-builder. This will be super-helpful for vscode-icons for github, no more manual downloading and building vscode-icons 👍

JimiC
JimiC reviewed Oct 14, 2017
View reviewed changes
docker/development/README.md Outdated
# Then run it.
# Note that we're using a volume to have access to the artifact in our host.
# Here we're using a Docker for Windows mapping. If you're using other OS then take a look at https://docs.docker.com/engine/admin/volumes/volumes/.
docker run -it -v d:/GIT/github/robertohuertasm/vscode-icons:/app dev sh
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be d:/GIT/github/vscode-icons/vscode-icons:/app ?

JimiC
JimiC reviewed Oct 14, 2017
View reviewed changes
docker/development/README.md Outdated
# Note that we're using a volume to have access to the artifact in our host.
# Here we're using a Docker for Windows mapping. If you're using other OS then take a look at https://docs.docker.com/engine/admin/volumes/volumes/.
# Once the container is done it will automatically be removed and the artifact will be in your mapped folder.
docker run --rm -it -v d:/GIT/github/robertohuertasm/vscode-icons:/app dev
Copy link
Member

@JimiC JimiC Oct 14, 2017 •
edited

Choose a reason for hiding this comment

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

Shouldn't this be d:/GIT/github/vscode-icons/vscode-icons:/app ?

JimiC
JimiC reviewed Oct 14, 2017
View reviewed changes
docker/remote-builder/README.md Outdated
# Note that we're using a volume to have access to the artifact in our host.
# Here we're using a Docker for Windows mapping. If you're using other OS then take a look at https://docs.docker.com/engine/admin/volumes/volumes/.
# Once the container is done it will automatically be removed and the artifact will be in your mapped folder.
docker run --rm -it -v c:/Users/Roberto/Desktop/s/shared:/app-out vsi
Copy link
Member

Choose a reason for hiding this comment

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

How about replacing Roberto with a placeholder?

@JimiC JimiC dismissed their stale review October 14, 2017 14:54

Awaiting reply on comments

@robertohuertasm robertohuertasm merged commit f4e44ee into master Oct 15, 2017
@JimiC JimiC deleted the docker branch October 15, 2017 10:03
@MarcelRaschke MarcelRaschke mentioned this pull request Nov 5, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Nov 5, 2022
Open
@MarcelRaschke MarcelRaschke mentioned this pull request Nov 5, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jens1o jens1o jens1o left review comments

@JimiC JimiC JimiC approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@robertohuertasm @JimiC @dderevjanik @jens1o