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 all rmw implementations #59

Merged
merged 2 commits into from
Nov 14, 2023
Merged

Install all rmw implementations #59

merged 2 commits into from
Nov 14, 2023

Conversation

christophebedard
Copy link
Member

Closes #58

@christophebedard christophebedard added the enhancement New feature or request label Nov 13, 2023
@christophebedard christophebedard self-assigned this Nov 13, 2023
@christophebedard christophebedard requested a review from a team as a code owner November 13, 2023 17:59
@christophebedard christophebedard requested review from emersonknapp and MichaelOrlov and removed request for a team November 13, 2023 17:59
@christophebedard christophebedard force-pushed the christophebedard/install-all-rmw-implementations branch from dc19028 to 7eb2aa0 Compare November 13, 2023 18:02
emersonknapp
emersonknapp approved these changes Nov 13, 2023
View reviewed changes
Copy link
Contributor

@emersonknapp emersonknapp left a comment

Choose a reason for hiding this comment

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

LGTM, though I don't think this will help steve's use case since he's using the empty base image (my recommended workflow, the variants are just an excuse to get your package.xml wrong)

@christophebedard christophebedard force-pushed the christophebedard/install-all-rmw-implementations branch from 7eb2aa0 to 86d78f4 Compare November 13, 2023 22:22
@christophebedard
Install all rmw implementations
d9117c9 
Signed-off-by: Christophe Bedard <[email protected]>
@christophebedard christophebedard force-pushed the christophebedard/install-all-rmw-implementations branch from 86d78f4 to d9117c9 Compare November 13, 2023 22:51
@christophebedard
Copy link
Member Author

Yeah. To satisfy #58 we'd need to install the rmw implementations in the base image, but then he might as well just install them right before the action-ros-ci step as he suggested here #58 (comment). I think installing them in the non-empty images and letting users of the empty images install them themselves makes sense.

@christophebedard christophebedard force-pushed the christophebedard/install-all-rmw-implementations branch from 49c639a to 26fcde6 Compare November 13, 2023 23:42
@christophebedard
Bump versions of actions
e0bd905 
Signed-off-by: Christophe Bedard <[email protected]>
@christophebedard christophebedard force-pushed the christophebedard/install-all-rmw-implementations branch from 26fcde6 to e0bd905 Compare November 13, 2023 23:42
@christophebedard christophebedard merged commit 185187f into master Nov 14, 2023
14 checks passed
@christophebedard christophebedard deleted the christophebedard/install-all-rmw-implementations branch November 14, 2023 01:34
@christophebedard christophebedard mentioned this pull request Nov 14, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emersonknapp emersonknapp emersonknapp approved these changes

@MichaelOrlov MichaelOrlov Awaiting requested review from MichaelOrlov MichaelOrlov was automatically assigned from ros-tooling/reviewers

Assignees

@christophebedard christophebedard

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Install all Tier 1 RMWs in containers
2 participants
@christophebedard @emersonknapp