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

Updated class_loader QD #152

Merged
merged 2 commits into from
May 21, 2020
Merged

Updated class_loader QD #152

merged 2 commits into from
May 21, 2020

Conversation

ahcorde
Copy link

@ahcorde ahcorde commented May 13, 2020

Updated class_loader QD

Signed-off-by: ahcorde ahcorde@gmail.com

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde self-assigned this May 13, 2020
QUALITY_DECLARATION.md Outdated Show resolved Hide resolved
QUALITY_DECLARATION.md Show resolved Hide resolved

### Direct Runtime ROS Dependencies [5.i]/[5.ii]

- console_bridge_vendor
- rcpputils
#### `console_bridge_vendor`
Copy link

Choose a reason for hiding this comment

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

@ahcorde meta: I'm a bit confused here, why do we depend on both the vendored package and the system dependency?

Copy link
Author

Choose a reason for hiding this comment

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

These are the run dependencies that are in the package.xml. yo system may work with the vendored or the system dependency. The QD need to include both

Copy link

Choose a reason for hiding this comment

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

Right, the point being that the vendored package will (should?) take care of that fallback behavior, no?

Copy link
Author

Choose a reason for hiding this comment

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

In other packages such as for example rcl_yaml_param_parser we also define both dependencies

Copy link
Author

Choose a reason for hiding this comment

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

You need to depend on both, if console_bridge doesn't exist then the vendor package will build it. I think this vendor package is only used for this. Then you need to depend on console_bridge to use the dependency.

QUALITY_DECLARATION.md Outdated Show resolved Hide resolved
QUALITY_DECLARATION.md Outdated Show resolved Hide resolved
QUALITY_DECLARATION.md Outdated Show resolved Hide resolved
Copy link

@brawner brawner left a comment

Choose a reason for hiding this comment

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

All of @hidmic's comments have been resolved, but it doesn't look like the commit to fix them has been pushed yet.

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde merged commit aadfdf9 into ros2 May 21, 2020
@clalancette clalancette deleted the ahcorde/fix/qd_table branch November 30, 2020 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants