Skip to content

Add user provided split size to error#430

Merged
piraka9011 merged 4 commits intoros2:masterfrom
aws-ros-dev:limit-test-throw
Jun 3, 2020
Merged

Add user provided split size to error#430
piraka9011 merged 4 commits intoros2:masterfrom
aws-ros-dev:limit-test-throw

Conversation

@piraka9011
Copy link
Copy Markdown
Contributor

  • Add the user defined split size to the splitting error.
  • Address comment on catching only std::runtime_error in tests.

Signed-off-by: Anas Abou Allaban allabana@amazon.com

Signed-off-by: Anas Abou Allaban <allabana@amazon.com>
@piraka9011
Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor

@zmichaels11 zmichaels11 left a comment

Choose a reason for hiding this comment

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

LGTM
Do you need a unit test for SequentialCompressionWriter to verify it throws when bad input is passed?

@piraka9011
Copy link
Copy Markdown
Contributor Author

piraka9011 commented Jun 3, 2020

LGTM
Do you need a unit test for SequentialCompressionWriter to verify it throws when bad input is passed?

Nope, it's already there.
It is not there. Adding.

Copy link
Copy Markdown
Contributor

@dabonnie dabonnie left a comment

Choose a reason for hiding this comment

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

Nice additions

Signed-off-by: Anas Abou Allaban <allabana@amazon.com>
@piraka9011
Copy link
Copy Markdown
Contributor Author

piraka9011 commented Jun 3, 2020

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

Signed-off-by: Anas Abou Allaban <allabana@amazon.com>
Signed-off-by: Anas Abou Allaban <aabouallaban@pm.me>
@piraka9011 piraka9011 merged commit 3e1cd8e into ros2:master Jun 3, 2020
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.

3 participants