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.

Sign up for GitHub

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

Jump to bottom

Inform users to install zlib when following windows binaries guide #3542

Merged
merged 2 commits into from
May 9, 2023
Merged

Inform users to install zlib when following windows binaries guide #3542

merged 2 commits into from
May 9, 2023

Conversation

Yadunund
Copy link
Member

@Yadunund Yadunund commented May 8, 2023

Without zlib added to PATH, the following error is thrown when running rviz2 on Windows from binaries. osrf/ros2_test_cases#863 (comment)

If users follow the source installation guide, they will be instructed to install zlib (among other deps) here. However the binary installation only links users to the Windows-Install-Prerequisites page that does not include these instructions.

Instead of having duplicate text in this doc as I have in the PR so far, let me know if a ref to the source installation instructions is preferred.

@Yadunund
Inform users to install zlib
36efcf8 
Signed-off-by: Yadunund <yadunund@openrobotics.org>
@Yadunund Yadunund mentioned this pull request May 8, 2023
Closed
1 task
@pradyum pradyum mentioned this pull request May 8, 2023
Closed
1 task
clalancette
clalancette requested changes May 9, 2023
View reviewed changes
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.

I think the addition here is fine, but given that this file is included in both the Binary and Source instructions, I think we should also remove it from the Source instructions. That way it will be in both of them exactly once.

@Yadunund
Remove zlib instructions from source setup
72aa5ba 
Signed-off-by: Yadunund <yadunund@openrobotics.org>
@Yadunund
Copy link
Member Author

Yadunund commented May 9, 2023

Nice catch!

@clalancette clalancette added the backport-iron backport at reviewers discretion; from rolling to iron label May 9, 2023
@clalancette clalancette merged commit a64e887 into rolling May 9, 2023
@delete-merged-branch delete-merged-branch bot deleted the yadu/zlib_for_windows_binaries branch May 9, 2023 13:45
mergify bot pushed a commit that referenced this pull request May 9, 2023
@Yadunund @mergify
Inform users to install zlib when following windows binaries guide (#…
fb031c9 
…3542)

* Inform users to install zlib

Signed-off-by: Yadunund <yadunund@openrobotics.org>
(cherry picked from commit a64e887)
@mergify mergify bot mentioned this pull request May 9, 2023
Merged
clalancette pushed a commit that referenced this pull request May 9, 2023
@mergify @Yadunund
Inform users to install zlib when following windows binaries guide (#…
35978d2 
…3542) (#3564)

* Inform users to install zlib

Signed-off-by: Yadunund <yadunund@openrobotics.org>
(cherry picked from commit a64e887)

Co-authored-by: Yadu <yadunund@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clalancette clalancette clalancette approved these changes

@audrow audrow Awaiting requested review from audrow audrow is a code owner

Assignees
No one assigned
Labels
backport-iron backport at reviewers discretion; from rolling to iron
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Yadunund @clalancette