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

Install fastjsonschema everywhere. #782

Merged
merged 1 commit into from
Jun 13, 2024
Merged

Install fastjsonschema everywhere. #782

merged 1 commit into from
Jun 13, 2024

Conversation

clalancette
Copy link
Contributor

We are going to need it for an upcoming change to rosidl.

@clalancette clalancette mentioned this pull request Jun 11, 2024
Merged
@clalancette
Copy link
Contributor Author

CI for this is over in ros2/rosidl#809 (comment)

wjwwood
wjwwood approved these changes Jun 12, 2024
View reviewed changes
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

lgtm, I guess this is covered by rosdep install in the manual instructions?

@clalancette
Copy link
Contributor Author

lgtm, I guess this is covered by rosdep install in the manual instructions?

Yep, exactly. Though you bring up a good point; for Windows installation instructions, we'll need to add it, since we don't use rosdep there. See ros2/ros2_documentation#4533

@clalancette
Copy link
Contributor Author

And actually, here is a RHEL-8 canary build, because now I suspect this won't actually work there:

  • RHEL-8 Build Status

@clalancette
Copy link
Contributor Author

OK, yeah. I'm going to have to do something slightly different here that requires another PR. Upcoming.

@clalancette clalancette force-pushed the clalancette/switch-to-fastjsonschema branch from 6f3342b to 04dee53 Compare June 12, 2024 19:57
@clalancette
Copy link
Contributor Author

All right, we now need ros-infrastructure/ros2-cookbooks#71 as well, which should fix this for both Windows and RHEL-8.

@clalancette
Copy link
Contributor Author

And here is RHEL-8 again with these changes:

  • RHEL-8 Build Status

@clalancette
Install fastjsonschema everywhere.
f47b9e4 
We are going to need it for an upcoming change to rosidl.

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
@clalancette clalancette force-pushed the clalancette/switch-to-fastjsonschema branch from 04dee53 to f47b9e4 Compare June 13, 2024 18:39
@clalancette
Copy link
Contributor Author

Updated with the latest hash from ros2-cookbooks now that ros-infrastructure/ros2-cookbooks#71 was merged. With that, going ahead and merging.

@clalancette clalancette merged commit 3a18e88 into master Jun 13, 2024
1 check passed
@clalancette clalancette deleted the clalancette/switch-to-fastjsonschema branch June 13, 2024 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wjwwood wjwwood wjwwood approved these changes

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

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

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

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