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

Allow separate launch composition #77

Merged
merged 5 commits into from
Dec 4, 2019
Merged

Allow separate launch composition #77

merged 5 commits into from
Dec 4, 2019

Conversation

oceanusxiv
Copy link
Contributor

As mentioned in #75, this is my attempt to allow composing nodes in different launch files. This change allows this by making target_container also a substitution so users can pass in the container node name. That way the container node itself can be executed first in another launch file.

Signed-off-by: Eric Fang <eric1221bday@gmail.com>
Signed-off-by: Eric Fang <eric1221bday@gmail.com>
@ivanpauno ivanpauno self-assigned this Sep 23, 2019
.gitignore Outdated Show resolved Hide resolved
Signed-off-by: Eric Fang <eric1221bday@gmail.com>
Signed-off-by: Eric Fang <eric1221bday@gmail.com>
@oceanusxiv
Copy link
Contributor Author

@ivanpauno so is this good?

@ivanpauno
Copy link
Member

@ivanpauno so is this good?

Yes, it's good.
We can follow-up about is_a_subclass usage later (#77 (comment)).


We're currently in a feature freeze, so this is not going to be merged into master until we create an Eloquent branch (probably in one/two weeks).

@oceanusxiv
Copy link
Contributor Author

@ivanpauno gentle ping now that the feature freeze seems to be over.

Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

Thanks for the reminder @eric1221bday

CI (using rebased branch ivanpauno/test-#77):

  • Linux Build Status
  • Linux-aarch64 Build Status

.gitignore Outdated
@@ -4,3 +4,4 @@ __pycache__
.mypy_cache
.pytest_cache
.flake8

Copy link
Member

Choose a reason for hiding this comment

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

nit: delete unnecessary change.

@ivanpauno
Copy link
Member

@eric1221bday please address flake8 failures.

Signed-off-by: Eric Fang <eric1221bday@gmail.com>
@oceanusxiv
Copy link
Contributor Author

@ivanpauno should be fixed

@ivanpauno
Copy link
Member

ivanpauno commented Dec 4, 2019

Sanity check (using rebased branch ivanpauno/test-#77):

  • Linux Build Status
  • Linux-aarch64 Build Status
  • MacOS Build Status
  • Windows Build Status

@ivanpauno
Copy link
Member

Going in! Thanks for the contribution @eric1221bday !

@ivanpauno ivanpauno merged commit 4662df0 into ros2:master Dec 4, 2019
@jacobperron
Copy link
Member

I'd like to see this change backported to Eloquent.

Since it looks like mainly bug fixes since the last release, maybe we can sync the eloquent branch with master?

@ivanpauno
Copy link
Member

Since it looks like mainly bug fixes since the last release, maybe we can sync the eloquent branch with master?

Sounds reasonable to me.

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.

None yet

7 participants