-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add dev container support for project #704
Add dev container support for project #704
Conversation
This is so cool! Thank you @Joshmantova — I have no experience with this type of container. Any reason I should wait before merging this? It looks reasonable to me. |
Nope! It's good to go unless you have additional suggestions for developer environments. We can always add to it later too. Now that I know you're interested in this, I'll throw up another pr to update the contributor documentation with further instructions. |
I realized there were pre-commit hooks in this project so I added those as well. Had to add pre-commit as a dependency as well. Also changed pre-commit python version to 3.11 (this is also what the devcontainer will load in). |
.devcontainer/devcontainer.json
Outdated
"ghcr.io/devcontainers/features/github-cli:1": {}, | ||
"ghcr.io/schlich/devcontainer-features/powerlevel10k:1": {} | ||
}, | ||
"image": "mcr.microsoft.com/devcontainers/python:3.11", |
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 uses Python 3.11 - would the team like me to use 3.9 instead?
@@ -8,7 +8,7 @@ on: | |||
types: [opened, synchronize, reopened] | |||
|
|||
env: | |||
POETRY_VERSION: "1.6.1" | |||
POETRY_VERSION: "1.7.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.
In the poetry lock file, it looks like it's getting compiled with 1.7.1
- I'd recommend updating the version in CI to reflect what developers are using locally.
This looks great! Thanks for doing this work. How hard would it be to create a CI step to make sure the container runs properly? If its too difficult, let me know and I can merge this in. |
@isaacbmiller - That's a great idea! Given that it takes a few minutes to build the container, we'd probably only want to trigger the CI workflow if the dev container files have changed. I'll get working on that but I'd love for it to be a fast follow so I don't need to keep resolving merge conflicts. Given that the changes here only affect internal developers using dev containers, which is currently only myself, I think it's safe to merge this in and bring in the CI step afterward. What do you think about that plan? |
In response to some of @thomasahle 's comments in Discord, I decided to downgrade the dev container to python3.9 from 3.11. He pointed out that backward compatibility is essential and because of that, we should focus on testing using 3.9 rather than 3.11. If we're going to focus on 3.9, we should also recommend internal developers use 3.9 and use that in the dev container. Now there are no substantive changes to CI and the only changes here are relevant to the dev container. This comes with the exception of upgrading the Poetry version used in CI to reflect the version developers are using locally to compile the lock file. |
Hey folks, when you set this one up did you able to run the notebooks locally?
|
@ammirsm Works for me! When I ran the first cell, VSCode asked me to install an additional extension automatically. Make sure you have the correct venv selected for your notebook. If you're getting an error, could you send me the error either in discord or create an issue with the error and tag me in it? |
Is that running all blocks with cache? Feel free to shoot me dm on discord with the same username on here |
Currently, there's a contributor.md file that looks to be out of date. I think it's confusing for new contributors to join and meaningfully contribute to the project because getting set up is somewhat complicated. This introduces a VSCode dev container solution for all developers to share a dev environment including proper linting, requirements, and extensions. I think this will make it easier for additional contributors to join without friction and improve the project. Let me know if there are any other customizations you'd want developers to be set up with for this project. Because this repo is on github, this will also allows anyone to develop from the web browser using Github codespaces and still have a proper dev environment set up.