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

README: add '-b <DISTRO>' to git clone instruction #358

Closed
wants to merge 1 commit into from

Conversation

JEnoch
Copy link
Contributor

@JEnoch JEnoch commented Dec 23, 2024

Now that there are separate branches for each distribution and the rolling branch can no longer be compiled on Jazzy, the build instructions need to be updated to guide users to clone the correct branch.

@@ -23,7 +23,7 @@ See [zenoh_cpp_vendor/CMakeLists.txt](./zenoh_cpp_vendor/CMakeLists.txt) for mor

```bash
mkdir ~/ws_rmw_zenoh/src -p && cd ~/ws_rmw_zenoh/src
git clone https://github.com/ros2/rmw_zenoh.git
git clone https://github.com/ros2/rmw_zenoh.git -b <DISTRO>
Copy link
Contributor

Choose a reason for hiding this comment

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

If you make it -b $ROS_DISTRO it's just as readable but also copy-pastable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree! I was just complying with the other instructions below which are using <DISTRO> instead of $ROS_DISTRO.

@Yadunund any reason for not having used $ROS_DISTRO in the very first version of this README ?

Copy link
Member

Choose a reason for hiding this comment

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

Probably because we don't source the distro till a couple steps after so the ROS_DISTRO envar will not be set at this point.

Anyways, I have the -b flag added in this PR #356 so we can close this PR out.

@Yadunund Yadunund closed this Dec 23, 2024
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