use ament_cmake_export_interfaces

[dirk-thomas]

Exports the created library target using ament_cmake_export_interfaces.

[dirk-thomas]

• 
• 
• 

[dirk-thomas]

Waiting for review.

[wjwwood]

What is the purpose of the pr? There's no context here.

[dirk-thomas]

The purpose is to export the library target (instead / beside exporting the include directory and library path). Then the downstream package can use it as an imported target (see referenced PR). This is the "new" way in CMake compared to the "old" way.

[dirk-thomas]

Using imported targets is a cleaner approach to pass information along to downstream packages. Compared to the classic approach of using *_INCLUDE_DIRS, *-LIBRARIES a downstream package can now:

• link against a specific library if an upstream package provides multiple
• link against the imported target without worrying what is "attached" to it (include dirs, transitive libraries, transitive include dirs, definition, compile flags, anything)
• information are not expanded to absolute paths until they are being used in a downstream package

While ament supported this for a long time we have never used it until now. Imo existing code should in the future move to this pattern instead of the classic approach to leverage these advantages.

[wjwwood]

While ament supported this for a long time we have never used it until now. Imo existing code should in the future move to this pattern instead of the classic approach to leverage these advantages.

I understand and agree it's a cool feature and a better way to expose information, but what you still haven't answered, imo, is why now and why this target in this package rather than all packages or N packages or something else? The motivation seems to be "because".

[dirk-thomas]

I understand and agree it's a cool feature and a better way to expose information, but what you still haven't answered, imo, is why now and why this target in this package rather than all packages or N packages or something else? The motivation seems to be "because".

I try to squeeze in time for reducing technical debt whenever I can. It just happened to be the case that I picked this item (exporting / importing targets). I picked this package since it has exactly one downstream dependency and was therefore easy to convert.

[wjwwood]

I try to squeeze in time for reducing technical debt whenever I can. It just happened to be the case that I picked this item (exporting / importing targets). I picked this package since it has exactly one downstream dependency and was therefore easy to convert.

Ok, that's fine, but in the future, if you want people to review it quickly, I'd recommend you add some context. I hesitated at the time you opened them because I didn't know if it was related to the console logging work or some other issue you were debugging and I didn't know if more was to come. If it's just trying it out or burning down some technical debt, just say so.

[dirk-thomas]

I hesitated at the time you opened them because I didn't know if it was related to the console logging work or some other issue you were debugging and I didn't know if more was to come.

I understand the "in review" label that it means that a PR is ready to be reviewed and mergable. If that is not yet the case the ticket explicitly mentions something like "just for feedback" or "please review but more is coming".

[wjwwood]

I'm just telling you why I didn't review it immediately, which was there was no context or motivation so I was uncertain as to its purpose or priority. I guess others felt the same way as no one else reviewed it. You could avoid this in the future by being more generous with details from the onset. It's just advice you don't have to follow it.