-
Notifications
You must be signed in to change notification settings - Fork 30k
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
build: add devcontainer configuration #40825
Conversation
Signed-off-by: Tierney Cyren <[email protected]>
Signed-off-by: Tierney Cyren <[email protected]>
I should also add: The intent of adding these files is to allow for easier direct development of Node.js. It means that when you try to launch Codespaces from nodejs/node in GitHub, you'd be set up with the environment defined in these configuration files rather than a raw default environment with a git clone of the source and nothing else. It also means that you'd be able to effectively do the same with Docker Desktop's Development Environment preview and any future implementors of the devcontainer config format (GitPod, for example, has this as a roadmap item). These configurations are not required to use the Docker image from nodejs/admin#641 as a developer environment. Rather, they take the image built from that repo and extend it slightly to improve the experience outside of those who want to figure out how to launch the Docker container that repo builds locally or how to spin it up as a chonky VM in the cloud and SSH in to it. |
} | ||
} | ||
} | ||
} |
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.
Nit: The indentation in this file is inconsistent.
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.
fixed ty
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.
actually fixed now :P
Signed-off-by: Tierney Cyren <[email protected]>
is this pr still relevant? I am using GitHub codespace for node source code development, but we have no devcontainer configuration to config the docker env of GitHub codespace so I have to install ninja manually. |
This PR is still relevant (imo). It was stalled for a long time waiting on us to get a Docker org. We now have that. Unfortunately around that time I was feeling pretty drained and was beginning to feel the need to job search. I've now job searched, quit my job, and am on a brief mental health recovery period that was graciously agreed to by my new employer before working again. This is maybe like 5 hours of work from being complete, not including figuring out who in the org owns the Node.js Docker Hub org and setting that governance up. Codespaces does work now, but kinda sucks as you mention. The Docker image has been transferred into the org but isn't being built/deployed presently. I'll be able to spend some time on this in the back half of June! Also happy to pair on it at Summit if anyone would like that. |
Signed-off-by: Tierney Cyren <[email protected]>
some small updates from airplane driven development:
|
tl;dr update on this:
I'd love to try to get that last part done while in-person at OpenJS World in Austin. It should be extremely do-able. If we can unblock that, we should be good to get the nodejs/devcontainer repo automated, let it simmer for a bit and see if there are any problems, and then from there merge this PR. |
Tagging with TSC agenda per a discussion I had with a TSC member: Currently, the only thing blocking this from landing (outside of the failures that shouldn't be failing?) is working with someone to get the Docker auth setup. The OpenJS Foundation staff have the keys to the org that we need, so the remaining task is just doing that manual work to get things set up. I'm happy to pair on this with anyone almost anytime in the next ~month since I'll have plenty of time. Just need to figure out who/when :) |
With Brian leaving OpenJS Foundation I do not know who to ask, but we can ask Robin to direct us. |
@nodejs/build are there any volunteers that would like to pair with Tierney? |
✋🏻 |
Removing from the TSC agenda, Tierney walked me through the setup. |
small status update: the repo (nodejs/devcontainer) seems to be working. Before taking her leave, Jory got me the necessary credentials to Make It Work (we should discuss making it more under our control at a later point - currently publishing from the Foundation account to the nodejs org on Docker Hub). It published the first container successfully. I want to let it publish nightly for at least a week without problems before considering requesting this PR be merged, but we shouldn't be far off of that now 👍🏻 |
Just a small update: The Docker image seems to have successfully built + published for the past 23 days! 🎉 I just followed the instructions and it also seems to be working exactly as one would hope (everyone who's going to use this might want to start working on a custom environment setup script to make it pretty how you like it): On this PR: I need to figure out what about the setup is currently killing all actions. Once I've done that, we should be good to go. |
4d90714 added a Git submodule ( |
Gotcha, I wouldn't have figured that out fast so thank you. I've literally never touched submodules so must have somehow done this accidentally and will figure out how to undo it 😅 |
Signed-off-by: Tierney Cyren <[email protected]>
3cc94c5
to
63e54ed
Compare
Force pushed the same changes hopefully sans submodule 🤞🏻 |
Signed-off-by: Tierney Cyren <[email protected]>
If tests pass, this should be mergable. Since this theoretically doesn't change any of Node.js itself, tests should pass 👀 |
Putting this in draft mode and then putting it back in ready-for-review so that the tests can re-run. Current tests show failures that are almost certainly unrelated. |
Commit Queue failed- Loading data for nodejs/node/pull/40825 ✔ Done loading data for nodejs/node/pull/40825 ----------------------------------- PR info ------------------------------------ Title build: add devcontainer configuration (#40825) Author Tierney Cyren (@bnb) Branch bnb:bnb/add-devcontainer -> nodejs:main Labels meta Commits 6 - build: don't ignore devcontainer - build: add devcontainer configs - build: fix inconsistent spacing - chore: remove unnessary WORKDIR - build: tweaks to various devcontainer configs - build: remove erroneous devcontainer submodule Committers 1 - Tierney Cyren PR-URL: https://github.com/nodejs/node/pull/40825 Reviewed-By: James M Snell ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/40825 Reviewed-By: James M Snell -------------------------------------------------------------------------------- ⚠ Commits were pushed since the last review: ⚠ - build: tweaks to various devcontainer configs ⚠ - build: remove erroneous devcontainer submodule ℹ This PR was created on Mon, 15 Nov 2021 23:50:30 GMT ✔ Approvals: 1 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/40825#pullrequestreview-806724188 ✘ Last GitHub CI failed ℹ Green GitHub CI is sufficient -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/4773575905 |
PR-URL: #40825 Reviewed-By: James M Snell <[email protected]>
Landed in 0c5f253 |
PR-URL: nodejs#40825 Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#40825 Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#40825 Reviewed-By: James M Snell <[email protected]>
PR-URL: #40825 Reviewed-By: James M Snell <[email protected]>
PR-URL: #40825 Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#40825 Reviewed-By: James M Snell <[email protected]>
this PR adds two metadata files:
./.devcontainer/.devcontainer.json
provides metadata to GitHub Codespaces and other tools (like Docker Desktop's preview Developer Environments feature) about how to build and configure a developer environment../.devcontainer/Dockerfile
a very simpleDockerfile
that the.devcontainer.json
file uses to build from. This will consume the Docker image I've built out (pending move bnb/devenv to nodejs/devcontainer admin#641 on moving it into nodejs/devcontainer) that will be built and published nightly.These should add a relatively low maintenance burden - the
Dockerfile
is literally two LOC (most of the work for this is done in the nodejs/devcontainer repo!) and the.devcontainer.json
is effectively a configuration file that represents some settings. It could be stripped down or more thoroughly built out if folks want, though I'd generally like to request that we err on configuring for a broader audience than just project members (think the audience at a Code and Learn event) while also serving ourselves.This PR should not land until nodejs/admin#641 lands.