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

find_library: Centralize functionality here #25

Merged
merged 8 commits into from
Sep 11, 2019
Merged

find_library: Centralize functionality here #25

merged 8 commits into from
Sep 11, 2019

Conversation

dirk-thomas
Copy link
Member

@dirk-thomas dirk-thomas commented Sep 5, 2019 •
edited
Loading

Replaces #4.

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

EricCousineau-TRI and others added 8 commits September 5, 2019 15:14
@EricCousineau-TRI @dirk-thomas
find_library: Centralize functionality here
7da6d0a 
Signed-off-by: Eric Cousineau <[email protected]>
@EricCousineau-TRI @dirk-thomas
fix
cf2c620 
Signed-off-by: Eric Cousineau <[email protected]>
@EricCousineau-TRI @dirk-thomas
fix for win32
94cb115 
Signed-off-by: Eric Cousineau <[email protected]>
@EricCousineau-TRI @dirk-thomas
Ensure test_library exports symbols
1b0dc53 
Signed-off-by: Eric Cousineau <[email protected]>
@EricCousineau-TRI @dirk-thomas
Fix missing header
5f3a736 
Signed-off-by: Eric Cousineau <[email protected]>
@dirk-thomas
fix visibility macro
afcf91e 
Signed-off-by: Dirk Thomas <[email protected]>
@dirk-thomas
fix rcutils_get_env usage
ed7b9fa 
Signed-off-by: Dirk Thomas <[email protected]>
@dirk-thomas
use different name for local variable
1a3a57e 
Signed-off-by: Dirk Thomas <[email protected]>
@dirk-thomas dirk-thomas added enhancement New feature or request in review labels Sep 5, 2019
@dirk-thomas dirk-thomas requested a review from a team as a code owner September 5, 2019 23:30
@dirk-thomas dirk-thomas self-assigned this Sep 5, 2019
@dirk-thomas dirk-thomas mentioned this pull request Sep 5, 2019
Closed
@dirk-thomas
Copy link
Member Author

Two PRs using this new location:

emersonknapp
emersonknapp approved these changes Sep 6, 2019
View reviewed changes
Copy link
Contributor

@emersonknapp emersonknapp left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for updating the previous one

@dabonnie
Copy link
Contributor

Do we need another reviewer to move this forward?

@dirk-thomas
Copy link
Member Author

Do we need another reviewer to move this forward?

Not anymore. I just changed the approval limit from 2 to 1 (since this was the only repo requiring two reviews and the people which set this up seem to not watch it).

@dirk-thomas dirk-thomas merged commit fa1c669 into master Sep 11, 2019
@delete-merged-branch delete-merged-branch bot deleted the dirk-thomas/move-find_library branch September 11, 2019 18:09
@dirk-thomas dirk-thomas mentioned this pull request Feb 3, 2020
Merged
@ivanpauno ivanpauno mentioned this pull request Feb 6, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emersonknapp emersonknapp emersonknapp approved these changes

Assignees

@dirk-thomas dirk-thomas

Labels
enhancement New feature or request in review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dirk-thomas @dabonnie @emersonknapp @EricCousineau-TRI