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

Included abstraction for rcutils::shared_library #49

Merged
merged 23 commits into from
Apr 2, 2020
Merged

Included abstraction for rcutils::shared_library #49

merged 23 commits into from
Apr 2, 2020

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Mar 20, 2020

Included abstraction for rcutils::shared_library. To be able to use it with C++

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

@ahcorde
Included abstraction for rcutils::shared_library
efc9963 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from a team as a code owner March 20, 2020 22:11
@ahcorde ahcorde mentioned this pull request Mar 20, 2020
Merged
wjwwood
wjwwood reviewed Mar 21, 2020
View reviewed changes
include/rcpputils/shared_library.hpp Outdated Show resolved Hide resolved
include/rcpputils/shared_library.hpp Outdated Show resolved Hide resolved
include/rcpputils/shared_library.hpp Outdated Show resolved Hide resolved
include/rcpputils/shared_library.hpp Outdated Show resolved Hide resolved
include/rcpputils/shared_library.hpp Outdated Show resolved Hide resolved
src/shared_library.cpp Outdated Show resolved Hide resolved
@ahcorde
Fixed headers and doc
a5c187a 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from wjwwood March 23, 2020 08:10
@ahcorde
Added basic test with sanitizers for shared libray
b19a01d 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
fixed linters in test_shared_library.cpp
96018c9 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
adapted to rcutils shared library API
7995d80 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
rcutils is a depend in package.xml
27c06f7 
Signed-off-by: ahcorde <ahcorde@gmail.com>
dirk-thomas
dirk-thomas reviewed Mar 23, 2020
View reviewed changes
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
include/rcpputils/shared_library.hpp Show resolved Hide resolved
include/rcpputils/shared_library.hpp Outdated Show resolved Hide resolved
test/test_shared_library.cpp Show resolved Hide resolved
dirk-thomas
dirk-thomas reviewed Mar 23, 2020
View reviewed changes
src/shared_library.cpp Outdated Show resolved Hide resolved
@ahcorde
Added get_library_path method to SharedLibrary class
6d26b5e 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
throw bad_alloc
94eef84 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde mentioned this pull request Mar 23, 2020
Closed
7 tasks
@ahcorde
Added exception to docblock
23600a0 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
fixed CMakeLists issues
d5e56a9 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Default value for DISABLE_SANITIZERS to OFF
4a128d9 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Added more tests to test_shared_library
2b18d0d 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde force-pushed the ahcorde/feature/shared_library branch from 5630628 to 2b18d0d Compare March 26, 2020 10:34
@ahcorde
Copy link
Contributor Author

ahcorde commented Mar 26, 2020

friendly ping @dirk-thomas @wjwwood

wjwwood
wjwwood reviewed Mar 26, 2020
View reviewed changes
src/shared_library.cpp Outdated Show resolved Hide resolved
src/shared_library.cpp Outdated Show resolved Hide resolved
src/shared_library.cpp Outdated Show resolved Hide resolved
src/shared_library.cpp Outdated Show resolved Hide resolved
@ahcorde
Included rcutils_get_error_string and rcutils_reset_error
8d36d58 
Signed-off-by: ahcorde <ahcorde@gmail.com>
dirk-thomas
dirk-thomas reviewed Mar 26, 2020
View reviewed changes
CMakeLists.txt Outdated Show resolved Hide resolved
dirk-thomas
dirk-thomas reviewed Mar 26, 2020
View reviewed changes
src/shared_library.cpp Outdated Show resolved Hide resolved
@ahcorde
Removed sanitizer in CMakeLists.txt
2d53380 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
get_library_path throws an exception - updated docs too
da90788 
Signed-off-by: ahcorde <ahcorde@gmail.com>
dirk-thomas
dirk-thomas reviewed Mar 27, 2020
View reviewed changes
test/test_shared_library.cpp Outdated Show resolved Hide resolved
test/test_shared_library.cpp Show resolved Hide resolved
@ahcorde
Removed comments and fixed test on shared library
bf08783 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Fixed cpplint test_shared_library
d8000c4 
Signed-off-by: ahcorde <ahcorde@gmail.com>
dirk-thomas
dirk-thomas approved these changes Mar 30, 2020
View reviewed changes
Copy link
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

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

With passing CI.

@ahcorde
Added target_compile_definitions for dummy_shared_library
d38b130 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Added get_platform_library_name to SharedLibrary
a883112 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Visibility of SharedLibray Class on windows
ea33567 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Avoid noreturn issue on windows
2e7f57d 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Avoid noreturn issue on windows on more tests
3455326 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Contributor Author

ahcorde commented Apr 1, 2020

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

CI is green @wjwwood or @dirk-thomas can you review last changes?

wjwwood
wjwwood approved these changes Apr 1, 2020
View reviewed changes
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

There's some inconsistency with punctuation in the doc strings, and I had some inline comments, but nothing major.

lgtm otherwise.

include/rcpputils/shared_library.hpp Outdated Show resolved Hide resolved
@ahcorde
fixed docbloc
2e97c45 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Contributor Author

ahcorde commented Apr 1, 2020

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

@wjwwood wjwwood merged commit 52f69e4 into ros2:master Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emersonknapp emersonknapp emersonknapp left review comments

@dirk-thomas dirk-thomas dirk-thomas approved these changes

@wjwwood wjwwood wjwwood approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ahcorde @wjwwood @emersonknapp @dirk-thomas