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

Fixes include folder installation. #220

Merged
merged 3 commits into from
Jun 15, 2022

Conversation

Signed-off-by: Franco Cipollone <[email protected]>
tfoote
tfoote previously approved these changes Jun 13, 2022
Copy link
Collaborator

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

LGTM. This won't pass CI until maliput/maliput_drake#23 or maliput/maliput#508 (review) are merged, released, and rebuilt.

@francocipollone
Copy link
Collaborator Author

francocipollone commented Jun 13, 2022

@francocipollone
Copy link
Collaborator Author

maliput and maliput_drake have been built already, It should be ok now to be re-triggered.
I tried to retrigger it but I am not sure if I am a bit rusty with jenkins or if I don't have the bits to retrigger it. :D @tfoote

@tfoote
Copy link
Collaborator

tfoote commented Jun 14, 2022

You can request a retry from Jenkins by commenting here like this.

@ros-pull-request-builder retest this please

@tfoote
Copy link
Collaborator

tfoote commented Jun 14, 2022

The build passed, but has linter errors which are why it shows error now.

@francocipollone
Copy link
Collaborator Author

The build passed, but has linter errors which are why it shows error now.

image

  • The GCC and Clang-Tidy warnings are because there are some deprecated attributes in maliput (related to old rules api) and therefore when using those and compiling maliput_malidrive, some warnings are raised.

Is it a requirement to fix all the warnings? What's the best approach when using marked-as-deprecated api?

CC: @agalbachicar

@francocipollone francocipollone force-pushed the francocipollone/remove_include_root_cmake_file branch from cbe4a2c to 0463a34 Compare June 15, 2022 20:39
@francocipollone
Copy link
Collaborator Author

Issues were solved via 221 and 222.
Build farm should be happy now.

When merging we should: Rebase and Merge

@francocipollone
Copy link
Collaborator Author

@francocipollone francocipollone requested review from tfoote and removed request for tfoote June 15, 2022 21:10
Copy link
Collaborator

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

I'm glad it wasn't too much to clear the warnings.

@francocipollone francocipollone merged commit 6f17be0 into main Jun 15, 2022
@francocipollone francocipollone deleted the francocipollone/remove_include_root_cmake_file branch June 15, 2022 21:30
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