Skip to content

Added missing dependency to typelib#14

Closed
m0rsch1 wants to merge 1 commit into
rock-cpp:masterfrom
m0rsch1:add-missing-dependency
Closed

Added missing dependency to typelib#14
m0rsch1 wants to merge 1 commit into
rock-cpp:masterfrom
m0rsch1:add-missing-dependency

Conversation

@m0rsch1

@m0rsch1 m0rsch1 commented Jul 20, 2026

Copy link
Copy Markdown

This is a commit back from xrock_gui_model/bin/xrock-resolve-ports to change back to rock-instantiate.
Don't know if this is needed but it has been flagged as a fix by @malter

I've openend the pull-request to possibly merge back.
But i am not yet sure if this is needed any why it had been inserted.

This is a commit back from xrock_gui_model/bin/xrock-resolve-ports
to change back to rock-instantiate.
Don't know if this is needed but it has been flagged as a fix by
@malter

Signed-off-by: Moritz Schilling <moritz.schilling@dfki.de>
@chhtz
chhtz requested a review from maltewi July 20, 2026 12:13
@chhtz

chhtz commented Jul 20, 2026

Copy link
Copy Markdown
Member

Can't tell either if this is necessary. Opinions? @maltewi (and maybe @pierrewillenbrockdfki)

@pierrewillenbrockdfki

Copy link
Copy Markdown
Contributor

If i got that right, this is the definitions of Orocos.apply_conf:
tools/orocos.rb/lib/orocos/configurations.rb: def self.apply_conf(task, path, names = ['default'], overrides = true)
The change just explicitly sets the overrides variable to its default.

I don't see where rock-instantiate would need typelib present. This is not c++ where you need to have the header present to inspect an object, and the inspection is done in another ruby file anyways.

@m0rsch1

m0rsch1 commented Jul 21, 2026

Copy link
Copy Markdown
Author

Well, the 'true' flag at the end is indeed just stating the default, but well the default might change :)
However, i think, the typelib requirement is the change of interest here.
But i dont know if its needed or not but @malter added it with the commit message
"Added missing require for xrock-resolve-ports" which is a copy of rock-instantiate.
Maybe some complex port type cannot be converted from a dynamic to a 'normal' port without it?

@chhtz

chhtz commented Jul 21, 2026

Copy link
Copy Markdown
Member

I guess if the require is needed somewhere else, it should be where it is needed.

If you currently don't have anything that breaks without this, I would close this for now. Feel free to re-open once you can reproduce an issue (or if @malter can clarify the need for this).

@chhtz chhtz closed this Jul 21, 2026
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.

3 participants