[doc] Small elaboration for Docker image usage #215
Conversation
130s
force-pushed
the
doc/212_docker_image
branch
from
5300340
to
26ce5d4
Compare
doc/index.rst
Outdated
| +++++++++++++++++++++++++++++++++++++++ | ||
|
|
||
| You can use `DOCKER_IMAGE` variable for this. | ||
|
|
There was a problem hiding this comment.
Perhaps we should add a note on the requirements that such an image has to meet:
- wstool
- rosdep; including repository set-up und python-pip
- catkin-tools
There was a problem hiding this comment.
rosdep; including repository set-up und python-pip
What is repository set-up? rosdep init && rosdep update?
| @@ -39,8 +39,13 @@ ROSWS=wstool | |||
| ici_time_end # init_ici_environment | |||
|
|
|||
| function catkin { | |||
| PATH_CATKIN=/usr/bin/catkin | |||
There was a problem hiding this comment.
Variable should be kept local. Local variables should be lowercase.
| local cmd=$1 | ||
| shift | ||
| if [ ! -f ${PATH_CATKIN} ]; then | ||
| echo "[ERR] ${PATH_CATKIN} not available. Make sure ${PKG_CATKIN} is installed. See also https://github.com/ros-industrial/industrial_ci/issues/216"; |
There was a problem hiding this comment.
The error function should be used instead. I will take care of the colors and the exit code.
There was a problem hiding this comment.
Yeah, I just couldn't find err was used in the same .sh file so I tried not to use it.
|
I took the chance to improve your warning code. |
mathias-luedtke
force-pushed
the
doc/212_docker_image
branch
from
de69a2e
to
30e8f20
Compare
|
@ipa-mdl thanks, your change looks good. |
There was a problem hiding this comment.
Something went wrong with the last to commit, 0b8edca is now duplicated.
doc/index.rst
Outdated
| You can pull any `Docker` image by specifying in `DOCKER_IMAGE` variable, as long as the following requirement is ment: | ||
|
|
||
| * `python-catkin-tools`, `python-pip`, `python-wstool` | ||
| * `python-rosdep` including rosdep repository set-up |
There was a problem hiding this comment.
rosdep repository is not required, but ROS repo set-up is.
130s
force-pushed
the
doc/212_docker_image
branch
from
6ff000f
to
dffe668
Compare
I don't see it duplicated (you may have already fixed it?) but now I don't see an issue? |
|
7bc5355 should not be part of this PR, it's the same patch.. |
130s
force-pushed
the
doc/212_docker_image
branch
from
dffe668
to
664e07c
Compare
|
@ipa-mdl sorry. I rebased and now it should look good. |
|
I think some commits could be squashed, but it is not required. |
…l#212). [doc] Wrong rst format. [doc] Clarify Docker image requirement.
See ros-industrial#216 for the issue example.
130s
force-pushed
the
doc/212_docker_image
branch
from
664e07c
to
9239771
Compare
|
Some are squashed and CI passed. Thanks @ipa-mdl |
Not sure how effective this change is but trying to address confusion such as #212.