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

chore(deps): update rules_cc to 0.1.1 #113

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

wep21
Copy link
Contributor

@wep21 wep21 commented Jan 20, 2025

#112 with updating boost

@wep21
Copy link
Contributor Author

wep21 commented Jan 20, 2025

@hofbi @mvukov I appreciate it if you could review this change.

@hofbi hofbi mentioned this pull request Jan 20, 2025
@wep21
Copy link
Contributor Author

wep21 commented Jan 20, 2025

need to update isAlive() to is_alive() in ros_comm because isAlive() of Thread is removed since python3.9.

@hofbi
Copy link
Contributor

hofbi commented Feb 5, 2025

@mvukov Could you update your forked branch for ros_comm used here to a more recent version (maybe even the latest 1.17) It should at least contain ros/ros_comm@5f13bb3 which fixes the isAlive issue that currently breaks the sames.

MODULE.bazel Outdated
@@ -23,7 +23,7 @@ bazel_dep(name = "glog", version = "0.7.1")
bazel_dep(name = "googletest", version = "1.15.2")
bazel_dep(name = "lz4", version = "1.9.4")
bazel_dep(name = "platforms", version = "0.0.10")
bazel_dep(name = "rules_cc", version = "0.1.0")
bazel_dep(name = "rules_cc", version = "0.0.17")
Copy link
Owner

Choose a reason for hiding this comment

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

This seems relevant for this PR, other changes should be in a separate PR.

Copy link
Contributor

@hofbi hofbi Feb 5, 2025

Choose a reason for hiding this comment

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

The other changes are required to this pass. If you want this change as a single commit, we would have to bump all boost first in a seperate PR.

The boost bumps contain the fixes after 0.1.0 was yanked.

Copy link
Contributor

Choose a reason for hiding this comment

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

Merging #114 first, should help to get the rules_cc bump exclusively.

Copy link
Owner

Choose a reason for hiding this comment

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

I just now see that this is a downgrade. Why do we need to downgrade?

Copy link
Contributor

Choose a reason for hiding this comment

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

0.1.0 was yanked on BCR. But a few days ago 0.1.1 was released so upgrading to 0.1.1 is also possible.

@hofbi
Copy link
Contributor

hofbi commented Feb 6, 2025

@wep21 could you update the PR please. The boost bumps are already merged.

@wep21
Copy link
Contributor Author

wep21 commented Feb 6, 2025

@hofbi updated

MODULE.bazel Outdated Show resolved Hide resolved
@wep21 wep21 changed the title chore(deps): update rules_cc to 0.0.17 chore(deps): update rules_cc to 0.1.1 Feb 9, 2025
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