Skip to content

Comments

Add devcontainer.json to simplify local documentation creation and build task for VS Code#3528

Closed
Bi0T1N wants to merge 2 commits intoros2:rollingfrom
Bi0T1N:devcontainer_vscode
Closed

Add devcontainer.json to simplify local documentation creation and build task for VS Code#3528
Bi0T1N wants to merge 2 commits intoros2:rollingfrom
Bi0T1N:devcontainer_vscode

Conversation

@Bi0T1N
Copy link
Contributor

@Bi0T1N Bi0T1N commented May 4, 2023

This adds a devcontainer.json which is based on the provided Dockerfile and at the moment it is only configured for Visual Studio Code. Thus it also adds a tasks.json to provide a default build task.
It helps to verify changes locally before committing 😄

Bi0T1N added 2 commits May 4, 2023 21:57
information at https://containers.dev/

Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>
run with Ctrl+Shift+B or select
from tasks menu via Ctrl+Shift+P

Signed-off-by: Bi0T1N <Bi0T1N@users.noreply.github.com>
@Bi0T1N Bi0T1N requested review from audrow and clalancette as code owners May 4, 2023 20:07
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

i am not necessarily against this enhancement, probably most of developers use vscode especially documentation development. with that expected condition, it would be better to verify on local environment before creating PR with vscode. but i am not sure about the policy, so would like to hear opinion.

if we take this PR, https://docs.ros.org/en/rolling/The-ROS2-Project/Contributing/Contributing-To-ROS-2-Documentation.html#building-the-site-locally should be also update to how to check with vscode?

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

After reviewing this, we have some concerns about this pull request.

  1. This configuration seems to install some arbitrary packages on developer machines. It will install extensions (though the user has to confirm this), and further it will install the esbonio package, which we don't know anything about.
  2. In general, this seems like an individual setup, rather than a project-wide setting. Otherw users of vscode might want to use a different set of extensions. Additionally, we could have similar configurations for emacs, vim, sublime, etc etc. And we don't want to maintain all of those.

@clalancette
Copy link
Contributor

Because of the reasons above, I'm going to close this pull request out. Thank you for the contribution.

@Bi0T1N
Copy link
Contributor Author

Bi0T1N commented May 13, 2023

1. This configuration seems to install some arbitrary packages on developer machines.  It will install extensions (though the user has to confirm this), and further it will install the `esbonio` package, which we don't know anything about.

It doesn't install packages on the developer machine. Everything is encapsulated in the Docker/container image and thus you can even use it with GitHub Codespaces in your webbrowser. esbonio is a language server for reStructuredText, also only installed in the image.

2. In general, this seems like an individual setup, rather than a project-wide setting.  Otherw users of vscode might want to use a different set of extensions.  Additionally, we could have similar configurations for emacs, vim, sublime, etc etc.  And we don't want to maintain all of those.

It's the basic setup with all tools you need to edit, create, view and finally commit your changes. If you want to have another extensions, simply add it and rebuild the container.
The other tools don't have support for container encapsulated development environments afaik thus adding configurations for them is way more complicated and needs much more effort and maintenance.

If you still don't want to accept it as proposed in this PR, would it be okay to add another chapter within the Contributing to ROS 2 Documentation page?

@fujitatomoya
Copy link
Collaborator

personally i am not inclined to take editor specific setting in the repository

  • not everyone uses that setting, that could be conflict with other setting. (developer dependencies) probably we would want to opposite path to take?
  • we might end up taking many editor settings and need to maintain.
  • developer can still use their editor and environment as they like. (e.g. I mostly use emacs and tags/scope, set up this env for me after pull the source code with alias.)

If you still don't want to accept it as proposed in this PR, would it be okay to add another chapter within the Contributing to ROS 2 Documentation page?

same reasons apply to this. i would not recommend this.

after all, i am not positive to take this approach, but open to discuss and hear more opinion.

thanks,

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.

3 participants