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

Remove Node option from Python template? #44

Closed
chrmarti opened this issue Nov 15, 2022 · 7 comments
Closed

Remove Node option from Python template? #44

chrmarti opened this issue Nov 15, 2022 · 7 comments
Assignees

Comments

@chrmarti
Copy link
Contributor

The Python template offers Node as one of its options which is redundant with Node being offered together with all other features. Is there a reason for having this as a template option?

https://github.com/devcontainers/templates/blame/2f1b259ed382b844846cb8d0e8e05946f3873f18/src/python/.devcontainer/devcontainer.json#L5

@samruddhikhandale
Copy link
Member

When these templates were in the old repo, they shared same docs with images. For having node handy, all the images have a reason similar to this this one 👇

Given JavaScript front-end web client code written for use in conjunction with a Python back-end often requires the use of Node.js-based utilities to build, this container also includes nvm so that you can easily install Node.js.

Looks like it was assumed that node is helpful with most of the platforms/runtimes. This is what I think from the docs & why I kept this option during migration.

@Chuxel / @bamurtaugh might know the real reason behind it.

@Chuxel
Copy link
Member

Chuxel commented Nov 15, 2022

Is there a problem we're trying to resolve here? e.g., has someone complained (I might have missed it)?

To break it down:

  1. Most use cases for the images at the time were to enable web and service development where node was used by tools to generate client-side content (e.g. via webpack) even if you weren't deploying that way. Having yarn and nvm there by default both enabled these common tools use cases.
  2. Image build performance is always a focus and removing the need to install yarn and nvm helps here.
  3. Features didn't exist at the time.

Certainly, we could remove it from the template while still building yarn and nvm into the base image. Might that be a good way to get the best of all worlds?

@chrmarti
Copy link
Contributor Author

We recently added a lightweight way to create a dev container from scratch without a local folder or repository and there the additional and somewhat redundant step stood out and was brought to my attention. The UI steps are:

  • Pick the Python template.
  • Decide if and which Node version you want.
  • Pick any additional features (including Node).

Since Node (and many other potentially useful tools) are offered as features, it seems we can remove Node from the template options. Having Node as an option made sense before we had features. I haven't checked which of the other templates are doing the same, but we could look into simplifying these too.

@Chuxel
Copy link
Member

Chuxel commented Nov 17, 2022

Yeah I think we can remove it from the template - but we can still leave it in the image is my PoV. Retains the original functionality that way.

@bamurtaugh
Copy link
Member

+1 to removing - I think removing it would set the right precedent for other templates, where we want users to build up with features as much as possible.

@samruddhikhandale
Copy link
Member

There are few other templates which provides Node options in the UX. I'll remove those from Templates 👍

@samruddhikhandale
Copy link
Member

Closing as fixed with #59 🚀

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

No branches or pull requests

4 participants