search

microsoft / vscode-dev-containers

NOTE: Most of the contents of this repository have been migrated to the new devcontainers GitHub org (https://github.com/devcontainers). See https://github.com/devcontainers/template-starter and https://github.com/devcontainers/feature-starter for information on creating your own!
https://aka.ms/vscode-remote
MIT License
4.72k stars 1.4k forks source link

Remove "forwardPorts" from definitions? #868

Open chrmarti opened 3 years ago

chrmarti commented 3 years ago

Now that auto port forwarding works reliably for Linux (by inspecting /proc), new ports trigger a notification and we have the Ports view, we could remove the "forwardPorts" property from most definitions. Exception being those ports that are being listened on before VS Code is up. That would simplify the devcontainer.json we generate.

/cc @bamurtaugh @Chuxel

bamurtaugh commented 3 years ago

We've commented it out from the remote-try-* samples as we've made progress on auto port forwarding - it's left as a reference above the other ports properties to show the various ports options folks can set in their containers (i.e. in remote-try-python: https://github.com/microsoft/vscode-remote-try-python/blob/main/.devcontainer/devcontainer.json#L42).

It looks like the definitions where it isn't already empty/commented out are:

Would the proposal be:

chrmarti commented 3 years ago

I would remove them where they are not needed. @Chuxel mentioned: "Its still useful in scenarios where the port is spun up before VS Code is connected. (e.g. SSH, VNC, etc)." Not sure if maybe portsAttributes could be used in these cases (to lead users towards the new property instead of the old).

bamurtaugh commented 3 years ago

I just tried commenting out forwardPorts from the PHP devcontainer.json, and I got the expected "Hello remote world!" output as I did when forwardPorts was included.

When I have a bit more time, I can go through the other definitions I linked above that have forwardPorts and verify if they behave the same if the property gets commented out, and then open a PR to: