-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Conversation
//cc: @cmendible @parkr for a review. @2percentsilk, @joshspicer, @bailey, @matthewisabel as FYI. |
// If image not yet published, there will be no repo digests, so set to N/A if that is the case | ||
let name, digest; | ||
try { | ||
const imageNameAndDigest = await asyncUtils.spawn('docker', ['inspect', "--format='{{index .RepoDigests 0}}'", image], { shell: true, stdio: 'pipe' }); | ||
[name, digest] = imageNameAndDigest.trim().split('@'); | ||
} catch(err) { | ||
if(err.result.indexOf('Template parsing error') > 0) { | ||
name = 'N/A'; | ||
digest = 'N/A'; | ||
} else { | ||
throw err; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI - This is a bug I ran into while testing history markdown generation locally (didn't happen if image already was published which is the situation normally when this is run).
To see what ends up in history, run:
build/vscdc info --registry mcr.microsoft.com --registry-path vscode/devcontainers --release main jekyll
The result will be in containers/jekyll/history/dev.md
Otherwise unrelated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me – left a few comments. Thank you!
# && apt-get -y install --no-install-recommends <your-package-list-here> | ||
|
||
# [Optional] Uncomment this line to install additional gems. | ||
# RUN gem install <your-gem-names-here> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, we recommend folks add gems to their Gemfile and use bundle install
to manage them.
|
||
- `mcr.microsoft.com/vscode/devcontainers/jekyll:0` | ||
- `mcr.microsoft.com/vscode/devcontainers/jekyll:0.0` | ||
- `mcr.microsoft.com/vscode/devcontainers/jekyll:0.0.1` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you intend to version the Jekyll container? Since it's not presently tied to a Jekyll or GitHub Pages version, and because the post-install script will still run bundle install
, it may not need to be versioned at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this versioning is for the image contents. But if we don't have a reason to release, we wouldn't bump the version number up.
At a minimum, there's monthly OS patching, so we'd update roughly once a month and that would get the latest of everything. That just increments the last digit (since we use semver). We do the same thing for all images in the repo. This is explained here.
The other thing this does is future proof for when/if we need to make breaking changes.
LANGUAGE=en_US | ||
|
||
# Install bundler, latest jekyll, and github-pages for older jekyll | ||
RUN gem install bundler jekyll github-pages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to work, installing both Jekyll 3 and Jekyll 4 as desired to speed up the bundle install
process.
Co-authored-by: Parker Moore <[email protected]>
Co-authored-by: Parker Moore <[email protected]>
…ners into clantz/jekyll-onboard
…vscode-dev-containers into clantz/jekyll-onboard
This implements #919 to generate a jekyll image. Like other images, it will include nvm and yarn, but does not include a specific version of Node.js in
base.Dockerfile
. Instead this is added in the userDockerfile
based on a version indevcontainer.json
. The user will be prompted for which version when this definition is added via Add Development Configuration Files. Unneeded files (likebase.Dockerfile
) are also filtered out in this case.To test the image alone:
nvm install lts/*
to ensure node installsOnce merged,
mcr.microsoft.com/vscode/devcontainers/jekyll:dev
will be published, and on release the inital version of the image will go (currently set to0.0.1
).