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

Add compression factory stubs #311

Merged
merged 5 commits into from
Mar 6, 2020
Merged

Add compression factory stubs #311

merged 5 commits into from
Mar 6, 2020

Conversation

piraka9011
Copy link
Contributor

@piraka9011 piraka9011 commented Mar 5, 2020 •
edited
Loading

Add the initial stubs of the compression_factory used to create de/compressors.
Part of #297.

Signed-off-by: Anas Abou Allaban aabouallaban@pm.me

@piraka9011
Add compression factory stubs
639d751 
Signed-off-by: Anas Abou Allaban <aabouallaban@pm.me>
@zmichaels11 zmichaels11 self-requested a review March 5, 2020 16:52
@piraka9011
Use strings for compression_format
10872e9 
Signed-off-by: Anas Abou Allaban <aabouallaban@pm.me>
Add Impl docs. Replace return with throw
677e28c 
Signed-off-by: Anas Abou Allaban <allabana@amazon.com>
Update docs
033e731 
Signed-off-by: Anas Abou Allaban <allabana@amazon.com>
@piraka9011
Add CompressionFactoryImpl docstrings
2505b97 
Signed-off-by: Anas Abou Allaban <aabouallaban@pm.me>
@piraka9011 piraka9011 merged commit e79efb0 into ros2:master Mar 6, 2020
@brawner
Copy link
Contributor

brawner commented Mar 6, 2020

This looks like it introduced a build regression on windows.

Build Status

15:25:34 [53.157s] C:\ci\ws\src\ros2\rosbag2\rosbag2_compression\src\rosbag2_compression\compression_factory_impl.hpp(37,16): error C2039: 'logic_error': is not a member of 'std' (compiling source file C:\ci\ws\src\ros2\rosbag2\rosbag2_compression\src\rosbag2_compression\compression_factory.cpp) [C:\ci\ws\build\rosbag2_compression\rosbag2_compression.vcxproj]
15:25:34 [53.172s] C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.24.28314\include\string(24): message : see declaration of 'std' (compiling source file C:\ci\ws\src\ros2\rosbag2\rosbag2_compression\src\rosbag2_compression\compression_factory.cpp) [C:\ci\ws\build\rosbag2_compression\rosbag2_compression.vcxproj]
15:25:34 [53.219s] C:\ci\ws\src\ros2\rosbag2\rosbag2_compression\src\rosbag2_compression\compression_factory_impl.hpp(37,1): error C2065: 'logic_error': undeclared identifier (compiling source file C:\ci\ws\src\ros2\rosbag2\rosbag2_compression\src\rosbag2_compression\compression_factory.cpp) [C:\ci\ws\build\rosbag2_compression\rosbag2_compression.vcxproj]
15:25:34 [53.422s] C:\ci\ws\src\ros2\rosbag2\rosbag2_compression\src\rosbag2_compression\compression_factory_impl.hpp(44,16): error C2039: 'logic_error': is not a member of 'std' (compiling source file C:\ci\ws\src\ros2\rosbag2\rosbag2_compression\src\rosbag2_compression\compression_factory.cpp) [C:\ci\ws\build\rosbag2_compression\rosbag2_compression.vcxproj]
15:25:34 [53.438s] C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.24.28314\include\string(24): message : see declaration of 'std' (compiling source file C:\ci\ws\src\ros2\rosbag2\rosbag2_compression\src\rosbag2_compression\compression_factory.cpp) [C:\ci\ws\build\rosbag2_compression\rosbag2_compression.vcxproj]
15:25:34 [53.453s] C:\ci\ws\src\ros2\rosbag2\rosbag2_compression\src\rosbag2_compression\compression_factory_impl.hpp(44,1): error C2065: 'logic_error': undeclared identifier (compiling source file C:\ci\ws\src\ros2\rosbag2\rosbag2_compression\src\rosbag2_compression\compression_factory.cpp) [C:\ci\ws\build\rosbag2_compression\rosbag2_compression.vcxproj]

@Karsten1987
Copy link
Collaborator

@piraka9011 please make sure to run CI before merging PRs.

@brawner brawner mentioned this pull request Mar 6, 2020
Closed
@dirk-thomas
Copy link
Member

@piraka9011 If this isn't being fixes before the end of today please revert the PR today to allow nightly jobs in the coming day to pass.

@Karsten1987
Copy link
Collaborator

I assume that an include is missing: https://en.cppreference.com/w/cpp/error/logic_error

@zmichaels11 zmichaels11 mentioned this pull request Mar 7, 2020
Merged
@zmichaels11
Copy link
Contributor

Potential fix: #314

@piraka9011
Copy link
Contributor Author

@piraka9011 please make sure to run CI before merging PRs.

Yes that was an error on my part. Jumped the gun on the merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zmichaels11 zmichaels11 zmichaels11 approved these changes

@dabonnie dabonnie dabonnie approved these changes

@prajakta-gokhale prajakta-gokhale prajakta-gokhale approved these changes

@emersonknapp emersonknapp Awaiting requested review from emersonknapp

@Karsten1987 Karsten1987 Awaiting requested review from Karsten1987

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

Successfully merging this pull request may close these issues.

None yet
7 participants
@piraka9011 @brawner @Karsten1987 @dirk-thomas @zmichaels11 @dabonnie @prajakta-gokhale