-
Notifications
You must be signed in to change notification settings - Fork 12
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
REPO: Add devcontainer configuraiton #465
base: master
Are you sure you want to change the base?
Conversation
Could you explain what this enables us to do, exactly? I see a lot of stuff related to vscode and what not and I'm not really sure what that has to do with the project. Does the pre-commit script fail if somebody doesn't have codespell installed? I think it should bail out gracefully if they don't. |
These files allow GitHub codespace to create a dev container with everything a dev needs to create a PR. It's very helpful for new contributors to have the tools all set (including more or less forcing format) It has no effect on local vscode, or other dev environments. |
Is that the same as this dev container? https://containers.dev/ It sounds to me like this enables spinning up a container with a development environment for doing work.
Aside from the format enforcing, is there anything else it does? For example, I'm not sure what's required to make a PR besides Git itself (and a browser to submit the PR itself). I looked up GitHub Codespaces and while it sounds cool and useful, I'm not really sure if it's a source repo's "job" to include tools to facilitate working on it to that extent. It kinds of reminds me of all the format files for various text editors that end up in repos, when those are really user-specific, not useful to the repo as a whole. But maybe I'm just out of touch with how these can be used :) Would like to hear what other people think about it, in any case. |
Codespace gives you a VSCode pre-configured devcontainer including all the "stuff" you need. If we ever get to adding tests, the dev environment tends to get more "difficult" to set up. |
That may be true, but I'm not sure it would be applicable to most users (for example, I don't use VSCode, I just use Notepad++... not that everyone does). If a lot of other developers here are using VSCode though, then I could see it being more applicable. That's why I'm curious what other folks think about this. It's not even just that it's not directly part of the project in a direct way, but more so I wonder where the line should be drawn between stuff that belongs in this repo and "related helpful tools" that should maybe go somewhere else. I have my own biases though, so I'll take a step back from this and maybe come back after some other thoughts are voiced. |
If we are adding pipeline jobs that check for formatting, then I think what this is trying to solve is - how does the developer know what to do to fix the formatting and get the job to pass? Maybe they can read the pipeline logs and then manually fix everything, but it would be nice if they could run the tool locally. Traditionally, they would have to go through the pain of installing and configuring the formatter themselves on their local machine. The containerized version solves this by providing a configuration file that describes how to install the environment, and certain IDEs will launch this for you. |
One more point on this: We can't disable the codespace feature. Github offers it on every repo, so why not configure it to just "work" with our repo. As a "part time" developer who switches between Python, C, MarkDown... I have found I can spend a lot less time configuring dev environments and more time writing and reviewing code. It may be just me :) |
d34e6bf
to
7b64f4e
Compare
This commit allows for a codespace to be created with the pre configured commit hooks, codespell, and clang-format.
7b64f4e
to
de619eb
Compare
This commit allows for a codespace to be created with pre-configured commit hooks as well as installing codespell, and clang-format in the container.
We will need #463 and #466 merged before the pre-commit hook will work properly.