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

Add a devcontainer configuration with Docker #4198

Merged
merged 24 commits into from
Jul 3, 2023

Conversation

masavini
Copy link
Contributor

@masavini masavini commented Mar 7, 2023

Description

Adding an option to configure a VS Code Dev Container.
Some choices were made to select a minimal VS Code configuration and extensions list, I hope you'll find them reasonable.
Bonus track: persistent .bash_history file.

Could fix #2580 but tests and documentation have not been updated yet (I'd like to get a feedback before working on them).

Copy link
Member

@browniebroke browniebroke left a comment

Choose a reason for hiding this comment

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

Waw, thanks for this great looking contribution! I'm not using VSCocde myself, but I know a lot of folks do, so that would be a great addition, plus for coding in the cloud.

I haven't looked in details, but I recently listed to a discussion about it on a podcast, and from what I gathered, the project is aspiring at not being VSCode-specific, but more of a spec for reproducible local dev environments.

With that in mind, I'm tempted to suggest to:

  1. Don't make an option for the devcontainer config, always include it
  2. Change the "use Pycharm" flag to an "editor" choice with PyCharm, VSCode in it.
  3. If VScode is selected, include the customizations.vscode part of the devcontainer config
  4. Try to not call things "vscode" unless they are really for that (e.g. the user in the Docker image)

I'm not sure about keeping local.yml config if use_docker=no and devcontainer=y... Maybe we can make it work when docker is chosen for now, and review the non-docker case later? Feels like it might be confusing to have Dockerfiles when docker was opted-out...

@masavini
Copy link
Contributor Author

masavini commented Mar 9, 2023

good suggestions, i'll try to implement them in the next few days...

by the way, how would you call the container user? as we all know, naming things is one of the two hard things in Computer Science :)

a final consideration about local.yml: how would you configure the dev container without it? maybe creating another docker-compose file inside .devcontainer? it could be possibile, but surely hard to maintain...

@browniebroke
Copy link
Member

how would you call the container user?

Good question 😄 maybe uncreatively devcontainer? Or is too confusing?

naming things is one of the two hard things in Computer Science :)

that quote is incomplete, it's missing off-by-one errors at the end 😂

a final consideration about local.yml: how would you configure the dev container without it?

I wouldn't bother configuring it in this case (at least in the initial version). I'd like to focus on the simpler use case for now. I might be over-cautious, but my concern is to get some issues opened along the lines of "I've opted for no Docker, but I still get a compose file". Your solution of providing an explicit option has the advantage of setting that expectation, but on the other hand, this project already has a lot of options, and I'm not convinced that this feature deserves a new one.

@masavini
Copy link
Contributor Author

ok, but if you remove all of the docker-compose file, you should at least provide a dockerfile to build the dev container base image. and if you provide a dockerfile when people has opted for no docker, the problem with their expectations remains the same, i guess...

maybe a painless solution could be adding a well VISIBILE comment section in local.yml telling people why the file is still there even if they opted for no docker...

what about dev-user for the dev container user name?

@browniebroke
Copy link
Member

if you remove all of the docker-compose file, you should at least provide a dockerfile to build the dev container base image

I think that's where our optinion differs... Not sure I was clear in my previous message, so I'll rephrase. Here is what I was suggesting:

  1. If user opts for docker, provide local.yml, Dockerfiles and devcontainer
  2. If user doesn't choose docker, remove all docker-related files (as we do now) and do NOT generate devcontainer support.

In the 2nd case, maybe we could look at providing a more basic Dockerfile inside of the .devcontainer folder, but at this point it's not clear to me how it should work, hence why I'm suggesting to NOT it for this initial PR.

As a user, when I'm opting-out of Docker, I'm expecting to configure some stuff on my host machine (Python virtualenv, Postgres, Redis, etc...), which cookiecutter-django won't do for me. I just don't see the use case for a devcontainer in this situation, but maybe I'm missing something.

what about dev-user for the dev container user name?

Sounds good to me 👍🏻

@masavini
Copy link
Contributor Author

masavini commented Mar 10, 2023

thank you for your clarification.
to summarize:
if a user opts in for docker, don't remove the .devcontainer folder and local.yml.
if a user selects vscode as preferred editor and opts in for docker, then configure the dev container with vs code settings and extensions.

but what if vscode is selected but docker is not? i may create a .vscode directory with some settings definition, but it doesn't make too much sense...

@browniebroke
Copy link
Member

what if vscode is selected but docker is not?

Indeed, that would not produce anything yet. However, having this option would enable us to implement more customisations for VS Code in the future. I'm thinking of what we have for PyCharm, but maybe that doesn't make sense in VSCode land...

PS: I'm not using VS Code for my Python projects, so let me know if I'm not making sense 😄

hooks/post_gen_project.py Outdated Show resolved Hide resolved
@@ -0,0 +1 @@
*.html
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain why we need this one? It seems a bit unrelated (I don't see prettier configured).

I'm asking because In the pre-commit config, we exclude Django templates from prettier in another way:

exclude: '{{cookiecutter.project_slug}}/templates/'

Maybe we should make the 2 exclusins more consistent?

Copy link
Member

@browniebroke browniebroke left a comment

Choose a reason for hiding this comment

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

I've tested it today and it's very cool. Thanks for that!

I think it's ready from my perspective.

@browniebroke browniebroke merged commit 1ed6d6e into cookiecutter:master Jul 3, 2023
11 checks passed
@browniebroke browniebroke changed the title VS Code Dev Container Add a devcontainer configuration with Docker Jul 3, 2023
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.

VS Code Dev Container setup
2 participants