Skip to content

Specify library destination for install targets.#1

Open
mbharatheesha wants to merge 1 commit intomainfrom
specify-lib-destination
Open

Specify library destination for install targets.#1
mbharatheesha wants to merge 1 commit intomainfrom
specify-lib-destination

Conversation

@mbharatheesha
Copy link
Collaborator

No description provided.

Signed-off-by: Mukunda Bharatheesha <mukunda.bharatheesha@nobleo.nl>
@mbharatheesha mbharatheesha requested a review from Timple February 4, 2025 17:14
@Timple
Copy link
Member

Timple commented Feb 4, 2025

You should point to the tier4 remote.

@mbharatheesha
Copy link
Collaborator Author

mbharatheesha commented Feb 4, 2025

I thought so as well and I will try doing it. Also, there are a few cosmetic improvements as well. For example,

  • instead of adding the LIBRARY DESTINATION per line, everything can be consolidated with one install call
  • the registration of the composable target can be improved to follow the standard convention of nebula::ros::<ComposableTarget> instead of just ComposableTarget
  • Add example launch to show the use of composable nodes

I was concerned these points might come up once I target this to tier4 main. And that would take longer to merge. Instead, if we merge here and have binaries available, then the upstream PR can take its time while we could continue testing. And once upstream is ready, we can quickly update our binaries.

Of course another way to look at it is we could argue that we can keep minimal delta by introducing this change only for now and propose the additional points as future work.

Thoughts?

@mbharatheesha
Copy link
Collaborator Author

mbharatheesha commented Feb 4, 2025

You should point to the tier4 remote.

Here it is:
tier4#261

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.

2 participants