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.

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

github: Add devcontainer.json for development #79

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

nakedible
Copy link
Contributor

Allows the easy usage of GitHub Codespaces for development.

Note: while this is meant to simplify the usage of GitHub Codespaces for development, the presence of a devcontainer.json file will trigger a suggestion in VS Code to use a container for development also on physical machines. Using a container for local development, when not necessary, may make the development experience worse - so the suggestion to use the container should be just dismissed if not wanted.

I do not personally consider this a big issue, but some might, so I wanted to point it out before this gets blindly merged.

@BurntSushi
Copy link
Owner

I'd somewhat prefer not to do this, and I especially am not a fan of hopping on to the dependabot mouse wheel personally. What is the cost of not doing this? Does it block you from working on Jiff?

@nakedible
Copy link
Contributor Author

The cost is that whenever I create a new codespace, I need to manually switch to a branch which has this config, then rebuild the codespace container, and then switch back to the branch I do work on, and not press the automatic suggestion to rebuild the container.

Just inconvenience, takes maybe 10 minutes if I haven't worked on this repo for a while. I don't mind much.

The dependabot stuff was automatically added by VS Code, but is totally optional. I don't expect there to be a need to touch devcontainer.json for a year or two, so it's not doing much.

The real benefit of this is that anybody at all can simply go to the repo in github, select from the code dropdown to spawn a codespace, and after a minute they can make changes in VS Code on the library and run cargo test and cargo bench without doing any setup actions, and regardless of the OS they are using. (Try it?)

But that's up to you to determine if you think it is a meaningful benefit. I don't mind either way.

I'll just close this pull if you decide to not need it, or if you want the dependabot stuff out, just let me know.

@BurntSushi
Copy link
Owner

I think I'm happy to bring in the devcontainer.json but without dependabot. I may back it out if it ends up causing trouble, but if it's just going to help folks use Codespaces, then that SGTM.

(I did try Codespaces once a while ago, but I found it disorienting. I'd probably only use it if setting up a development environment was a huge burden.)

@nakedible
Copy link
Contributor Author

nakedible commented Aug 6, 2024

Okay - one additional question: do you prefer to have the comments in devcontainer.json as added by VS Code as an example? I personally prefer not to have such as they end up out of date fast. So the only required line in the JSON is the image line. If you also prefer a minimal file, I'll make that change as well.

@BurntSushi
Copy link
Owner

I don't think I can really evaluate that to be honest. I defer to you. With that said, I think at least having a link to some kind of docs explaining the format would be nice.

Allows the easy usage of GitHub Codespaces for development.
@nakedible
Copy link
Contributor Author

I left the comments in as I think that's more customary and it gives pointers where to look.

Copy link
Owner

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

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

Thanks!

@BurntSushi BurntSushi merged commit bd2b826 into BurntSushi:master Aug 6, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants