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

Update docker instructions #351

Conversation

christophfroehlich
Copy link
Contributor

@christophfroehlich christophfroehlich commented Aug 20, 2023

Using the sphinx tabs plugin, we could improve the instructions using the docker container.

Choosing one tab switches all group-tabs at once (as known from docs.ros.org). What do you think? Did that only for example 1, but would apply the changes to the others, too.

image

olivier-stasse
olivier-stasse previously approved these changes Sep 26, 2023
Copy link
Collaborator

@olivier-stasse olivier-stasse left a comment

Choose a reason for hiding this comment

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

LGTM and tested.
This a nice addition.
I had to battle a little bit to understand how to build the documentation in a stand-alone mode. I will try to propose a PR on control.ros.org.

The way it was tested was to modify the script make_help_scripts/deploy_defines specifying the repo and the branch

@christophfroehlich
Copy link
Contributor Author

what was the problem with spinhx? I always clone the three subrepos manually or create symlinks into the folder.

@olivier-stasse
Copy link
Collaborator

olivier-stasse commented Sep 26, 2023

I simply changed

declare -A subrepo_url=( ["ros2_control"]="https://github.com/ros-controls/ros2_control" ["ros2_controllers"]="https://github.com/ros-controls/ros2_controllers" ["ros2_control_demos"]="https://github.com/ros-controls/ros2_control_demos" ["gazebo_ros2_control"]="https://github.com/ros-controls/gazebo_ros2_control" ["gz_ros2_control"]="https://github.com/ros-controls/gz_ros2_control")
declare -A subrepo_pr=( ["ros2_control"]=$ROS2_CONTROL_PR ["ros2_controllers"]=$ROS2_CONTROLLERS_PR ["ros2_control_demos"]=$ROS2_CONTROL_DEMOS_PR ["gazebo_ros2_control"]=$GAZEBO_ROS2_CONTROL_PR ["gz_ros2_control"]=$GZ_ROS2_CONTROL_PR)

to

declare -A subrepo_url=( ["ros2_control"]="https://github.com/ros-controls/ros2_control" ["ros2_controllers"]="https://github.com/ros-controls/ros2_controllers" ["ros2_control_demos"]="https://github.com/christophfroehlich/ros2_control_demos" ["gazebo_ros2_control"]="https://github.com/ros-controls/gazebo_ros2_control" ["gz_ros2_control"]="https://github.com/ros-controls/gz_ros2_control")
declare -A subrepo_pr=( ["ros2_control"]=$ROS2_CONTROL_PR ["ros2_controllers"]=$ROS2_CONTROLLERS_PR ["ros2_control_demos"]=improved_docker_description ["gazebo_ros2_control"]=$GAZEBO_ROS2_CONTROL_PR ["gz_ros2_control"]=$GZ_ROS2_CONTROL_PR)

in
https://github.com/ros-controls/control.ros.org/blob/master/make_help_scripts/deploy_defines

no need to clone locally the subrepos as it is done by the script.

I failed to do it by hand...

@olivier-stasse
Copy link
Collaborator

Otherwise for this PR, if you fix the conflict file we can merge it.

Previous commit was making the format test failed and was at the wrong place.
Copy link
Collaborator

@olivier-stasse olivier-stasse left a comment

Choose a reason for hiding this comment

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

LGTM - All tests are passing

@olivier-stasse olivier-stasse merged commit 07a897f into ros-controls:master Sep 27, 2023
18 checks passed
@olivier-stasse
Copy link
Collaborator

@christophfroehlich sorry I fixed some minor issues and did this by commiting directy on your branch. Hope this is fine.

@christophfroehlich christophfroehlich deleted the improve_docker_description branch September 27, 2023 08:48
@christophfroehlich
Copy link
Contributor Author

@Mergifyio backport humble

@mergify
Copy link
Contributor

mergify bot commented Sep 27, 2023

backport humble

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Sep 27, 2023
* Update docker instructions

* Update userdoc.rst

---------

Co-authored-by: Olivier Stasse <ostasse@laas.fr>
(cherry picked from commit 07a897f)
christophfroehlich added a commit that referenced this pull request Sep 27, 2023
* Update docker instructions

* Update userdoc.rst

---------

Co-authored-by: Olivier Stasse <ostasse@laas.fr>
(cherry picked from commit 07a897f)

Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>
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