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

Improve rosdep: 1) Friendlier to beginners, 2) Clarifying rosdep is ROS-agnostic #3719

Merged
merged 11 commits into from
Aug 4, 2023

Conversation

130s
Copy link
Contributor

@130s 130s commented Jun 6, 2023

Current rendition is great, then I noticed there might be some more things the beginners and experienced rosdep users like to know.

Issue

  • Giving an impression that rosdep is ros-dependent. It is actually widely applicable in non-ROS projects.
    • Not knowing rosdep being non-ros dependent matters IMO where the development involves non-ROS environment e.g. corporate, where people tend to end up inventing their own dependency management framework that runs alongside of rosdep, resulted in rosdep not receiving contribution.
  • Trying to make it friendlier to non-expert readers by clarifyig rosdistro website and some jargons.
  • Explanation of <*depend> is missing.

Solution approach

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.

overall, i am good to take this improvement, probably we can keep this open for more feedback.

source/Tutorials/Intermediate/Rosdep.rst Outdated Show resolved Hide resolved
source/Tutorials/Intermediate/Rosdep.rst Outdated Show resolved Hide resolved
source/Tutorials/Intermediate/Rosdep.rst Outdated Show resolved Hide resolved
source/Tutorials/Intermediate/Rosdep.rst Outdated Show resolved Hide resolved
source/Tutorials/Intermediate/Rosdep.rst Outdated Show resolved Hide resolved
source/Tutorials/Intermediate/Rosdep.rst Outdated Show resolved Hide resolved
source/Tutorials/Intermediate/Rosdep.rst Show resolved Hide resolved
source/Tutorials/Intermediate/Rosdep.rst Outdated Show resolved Hide resolved
source/Tutorials/Intermediate/Rosdep.rst Show resolved Hide resolved
source/Tutorials/Intermediate/Rosdep.rst Outdated Show resolved Hide resolved
@130s 130s force-pushed the improve-rosdep branch 2 times, most recently from 406efbc to e7d4f3a Compare June 29, 2023 16:19
@130s 130s requested a review from clalancette June 29, 2023 17:14
@clalancette clalancette self-assigned this Jun 29, 2023
@clalancette
Copy link
Contributor

@130s FYI, I spent some time and cleaned this up to be closer to what I think it should look like. What I did was to get rid of the separate "define_dependencies" file, and moved that content into Rosdep.rst. Once that was done, there was no need to move the Rosdep file, so I moved that back to its original location. Then I went through and did a bunch of editing, cleaning things up so most of the information is there but in a different order.

I think I've preserved the essence of what you've done, but please take a look and let me know what you think.

source/Tutorials/Intermediate/Rosdep.rst Show resolved Hide resolved
:local:

Author: Steve Macenski

This tutorial will explain how to manage external dependencies using ``rosdep``.

What is rosdep?
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding a why as well. The motivation for rosdep isn't always clear, leading to questions such as why rosdep instead of others? and When should I use rosdep and when should I use apt or pip?

Copy link
Contributor

Choose a reason for hiding this comment

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

I've added a couple of sentences about this in fcdc39d . Let me know what you think.

source/Tutorials/Intermediate/Rosdep.rst Outdated Show resolved Hide resolved
source/Tutorials/Intermediate/Rosdep.rst Show resolved Hide resolved
@130s
Copy link
Contributor Author

130s commented Aug 4, 2023

@clalancette I "approved" (if there were a button I'd hit it). Thanks for cleaning my PR.

In one of the links @gavanderhoorn referred to, I found that there's yet another good set of documentation about rosdep http://docs.ros.org/en/independent/api/rosdep/html/. It's "independent", which I assume means "not dependent on any ros1 distro", meaning the web location is still under ros1 hence ROS2 maintainers may not want this PR to refer to it. But if linking from this PR to it is not an issue I'd suggest we should link, in a hope we wouldn't duplicate rosdep documentation in the future.

@clalancette
Copy link
Contributor

In one of the links @gavanderhoorn referred to, I found that there's yet another good set of documentation about rosdep http://docs.ros.org/en/independent/api/rosdep/html/. It's "independent", which I assume means "not dependent on any ros1 distro", meaning the web location is still under ros1 hence ROS2 maintainers may not want this PR to refer to it. But if linking from this PR to it is not an issue I'd suggest we should link, in a hope we wouldn't duplicate rosdep documentation in the future.

Yes, good point. That is independent of any ROS distributions, so we can include a link here. I'll find a place to put it.

130s and others added 10 commits August 4, 2023 16:31
- Avoid giving an impression that `rosdep` is ros-dependent. It is actually widely applicable than ROS projects.
   - This matters IMO where the development involves non-ROS environment e.g. corporate, where people tend to end up inventing their own dependency management framework that runs alongside of `rosdep`, resulted in `rosdep` not being contributed.
- Trying to make it friendlier to non-expert readers by clarifyig `rosdistro` website and some jargons.
- Explanation of `<*depend>` is missing in ros2 doc set. Something detailed like http://docs.ros.org/en/melodic/api/catkin/html/howto/format2/catkin_library_dependencies.html
   - Thought I'd add those in here but ran out of time. I don't know if rosdep page is the best for such info anyways.
Signed-off-by: Chris Lalancette <[email protected]>
Signed-off-by: Chris Lalancette <[email protected]>
@clalancette clalancette added the backport-all backport at reviewers discretion; from rolling to all versions label Aug 4, 2023
@clalancette clalancette merged commit 00e2140 into ros2:rolling Aug 4, 2023
3 checks passed
mergify bot pushed a commit that referenced this pull request Aug 4, 2023
…OS-agnostic (#3719)

* Improve: rosdep

- Avoid giving an impression that `rosdep` is ros-dependent. It is actually widely applicable than ROS projects.
   - This matters IMO where the development involves non-ROS environment e.g. corporate, where people tend to end up inventing their own dependency management framework that runs alongside of `rosdep`, resulted in `rosdep` not being contributed.
- Trying to make it friendlier to non-expert readers by clarifyig `rosdistro` website and some jargons.
- Explanation of `<*depend>` is missing in ros2 doc set. Something detailed like http://docs.ros.org/en/melodic/api/catkin/html/howto/format2/catkin_library_dependencies.html
   - Thought I'd add those in here but ran out of time. I don't know if rosdep page is the best for such info anyways.

* Improve: rosdep: Add link to detail about the types of dependency

Co-authored-by: Tomoya Fujita <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
(cherry picked from commit 00e2140)
mergify bot pushed a commit that referenced this pull request Aug 4, 2023
…OS-agnostic (#3719)

* Improve: rosdep

- Avoid giving an impression that `rosdep` is ros-dependent. It is actually widely applicable than ROS projects.
   - This matters IMO where the development involves non-ROS environment e.g. corporate, where people tend to end up inventing their own dependency management framework that runs alongside of `rosdep`, resulted in `rosdep` not being contributed.
- Trying to make it friendlier to non-expert readers by clarifyig `rosdistro` website and some jargons.
- Explanation of `<*depend>` is missing in ros2 doc set. Something detailed like http://docs.ros.org/en/melodic/api/catkin/html/howto/format2/catkin_library_dependencies.html
   - Thought I'd add those in here but ran out of time. I don't know if rosdep page is the best for such info anyways.

* Improve: rosdep: Add link to detail about the types of dependency

Co-authored-by: Tomoya Fujita <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
(cherry picked from commit 00e2140)
@130s 130s deleted the improve-rosdep branch August 4, 2023 20:43
clalancette pushed a commit that referenced this pull request Aug 4, 2023
…OS-agnostic (#3719) (#3820)

* Improve: rosdep

- Avoid giving an impression that `rosdep` is ros-dependent. It is actually widely applicable than ROS projects.
   - This matters IMO where the development involves non-ROS environment e.g. corporate, where people tend to end up inventing their own dependency management framework that runs alongside of `rosdep`, resulted in `rosdep` not being contributed.
- Trying to make it friendlier to non-expert readers by clarifyig `rosdistro` website and some jargons.
- Explanation of `<*depend>` is missing in ros2 doc set. Something detailed like http://docs.ros.org/en/melodic/api/catkin/html/howto/format2/catkin_library_dependencies.html
   - Thought I'd add those in here but ran out of time. I don't know if rosdep page is the best for such info anyways.

* Improve: rosdep: Add link to detail about the types of dependency

Co-authored-by: Tomoya Fujita <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
(cherry picked from commit 00e2140)

Co-authored-by: Isaac Saito <[email protected]>
clalancette pushed a commit that referenced this pull request Aug 4, 2023
…OS-agnostic (#3719) (#3821)

* Improve: rosdep

- Avoid giving an impression that `rosdep` is ros-dependent. It is actually widely applicable than ROS projects.
   - This matters IMO where the development involves non-ROS environment e.g. corporate, where people tend to end up inventing their own dependency management framework that runs alongside of `rosdep`, resulted in `rosdep` not being contributed.
- Trying to make it friendlier to non-expert readers by clarifyig `rosdistro` website and some jargons.
- Explanation of `<*depend>` is missing in ros2 doc set. Something detailed like http://docs.ros.org/en/melodic/api/catkin/html/howto/format2/catkin_library_dependencies.html
   - Thought I'd add those in here but ran out of time. I don't know if rosdep page is the best for such info anyways.

* Improve: rosdep: Add link to detail about the types of dependency

Co-authored-by: Tomoya Fujita <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
(cherry picked from commit 00e2140)

Co-authored-by: Isaac Saito <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-all backport at reviewers discretion; from rolling to all versions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants