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

feat: add ecal dependency into package.xml #68

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: add ecal dependency into package.xml #68

wants to merge 2 commits into from

Conversation

wep21
Copy link
Contributor

@wep21 wep21 commented Sep 28, 2023 •
edited
Loading

  • add ecal dependency into package.xml because I register ecal into ros buildfarm

@wep21
feat: add ecal dependency into package.xml
f7e85c8 
Signed-off-by: wep21 <daisuke.nishimatsu1021@gmail.com>
@wep21
Copy link
Contributor Author

wep21 commented Sep 28, 2023

@FlorianReimold @JWhitleyWork Could you review this PR?

@wep21 wep21 force-pushed the add-ecal-dependency branch 2 times, most recently from 07f47c2 to f897c0d Compare September 28, 2023 03:03
@wep21
update ci to use ecal ros package
82e03d7 
Signed-off-by: wep21 <daisuke.nishimatsu1021@gmail.com>
@wep21 wep21 force-pushed the add-ecal-dependency branch from f897c0d to 82e03d7 Compare September 28, 2023 03:09
@FlorianReimold
Copy link
Member

From eCAL side I see nothing to complain about! 😊

@JWhitleyWork
Copy link
Contributor

I would like to at least wait until the Rolling sync is complete and that distribution passes CI before merging this.

@wep21
Copy link
Contributor Author

wep21 commented Sep 29, 2023

@JWhitleyWork

I would like to at least wait until the Rolling sync is complete and that distribution passes CI before merging this.

No problem, thanks.

@wep21 wep21 closed this Sep 29, 2023
@wep21 wep21 reopened this Sep 29, 2023
JWhitleyWork
JWhitleyWork approved these changes Sep 29, 2023
View reviewed changes
Copy link
Contributor

@JWhitleyWork JWhitleyWork left a comment

Choose a reason for hiding this comment

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

LGTM

@JWhitleyWork
Copy link
Contributor

@FlorianReimold The Iron build failure is due to a needed CI change. Everything else looks good on this PR AFAIC.

@FlorianReimold
Copy link
Member

@JWhitleyWork: Shall I merge this? It's still not compiling on iron, but there already is an issue for that (#69)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JWhitleyWork JWhitleyWork JWhitleyWork approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@wep21 @FlorianReimold @JWhitleyWork