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.

Sign up for GitHub

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

Jump to bottom

[dashing] Set allocator before goto fail. #540

Merged
merged 1 commit into from
Dec 5, 2019
Merged

[dashing] Set allocator before goto fail. #540

merged 1 commit into from
Dec 5, 2019

Conversation

BorjaOuterelo
Copy link

@BorjaOuterelo BorjaOuterelo commented Nov 28, 2019 •
edited
Loading

Fix #539

It should be added also in master and Eloquent

jacobperron
jacobperron reviewed Dec 2, 2019
View reviewed changes
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

rcl/src/rcl/init.c Show resolved Hide resolved
@jacobperron jacobperron self-assigned this Dec 2, 2019
@BorjaOuterelo
Set allocator before goto fail.
d0d08f2 
Signed-off-by: Borja Outerelo <borjaouterelo@gmail.com>
jacobperron
jacobperron approved these changes Dec 3, 2019
View reviewed changes
Copy link
Member

@jacobperron jacobperron left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

@jacobperron jacobperron changed the base branch from dashing to master December 3, 2019 19:15
@jacobperron jacobperron changed the base branch from master to eloquent December 3, 2019 19:16
@jacobperron jacobperron changed the base branch from eloquent to dashing December 3, 2019 19:16
@jacobperron
Copy link
Member

@BorjaOuterelo I just noticed that this PR is targeting dashing. It would be better to target master and then we can backport the changes for Eloquent and Dashing.

@jacobperron jacobperron mentioned this pull request Dec 3, 2019
Merged
@jacobperron
Copy link
Member

I've ported the change to the master branch in #546

@jacobperron jacobperron changed the title Set allocator before goto fail. [dashing] Set allocator before goto fail. Dec 3, 2019
@BorjaOuterelo
Copy link
Author

Shall I be worried for the CI fail?
http://build.ros2.org/job/Epr__rcl__ubuntu_bionic_amd64/77/ It seems more like an infrastructure issue.

@jacobperron
Copy link
Member

Shall I be worried for the CI fail?

No. The failing job is for Eloquent (it was accidentally triggered when I temporary changed the target of this PR).

@jacobperron
Copy link
Member

I'm waiting for #546 to be merged before this one.

@jacobperron jacobperron merged commit a1000f6 into ros2:dashing Dec 5, 2019
@Acuadros95 Acuadros95 deleted the allocator_fix branch June 6, 2023 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobperron jacobperron jacobperron approved these changes

Assignees

@jacobperron jacobperron

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@BorjaOuterelo @jacobperron