Skip to content

Fast-CDR external dependency Quality Declaration#360

Closed
Blast545 wants to merge 6 commits intomasterfrom
blast545/qd_fastcdr
Closed

Fast-CDR external dependency Quality Declaration#360
Blast545 wants to merge 6 commits intomasterfrom
blast545/qd_fastcdr

Conversation

@Blast545
Copy link
Copy Markdown
Contributor

@Blast545 Blast545 commented Apr 1, 2020

This is a quality declaration for eProsima Fast-CDR external dependency for ROS2 packages as described in the proposed REP-2004.

This declaration represents some thoughts regarding the current state of this external dependency and how this affect Quality Level 1 for ROS2 packages, feedback is well received.

Signed-off-by: Jorge Perez jjperez@ekumenlabs.com

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
Copy link
Copy Markdown
Contributor

@brawner brawner left a comment

Choose a reason for hiding this comment

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

It looks like a lot of my comments from the FastRTPS PR can also be applied here. Could you update this with the most current style and carry over any relevant changes from the other PR and then I'll take a deeper dive?

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@Blast545 Blast545 requested review from ahcorde and brawner May 1, 2020 14:32
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

LGTM, just a small change


As stated in their public repository, *eProsima FastCDR is a C++ library that provides two serialization mechanisms. One is the standard CDR serialization mechanism, while the other is a faster implementation that modifies the standard*.

## Summary
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For large paragraphs like these below, it might be better to have once sentence per-line so that the diff is easier to read.

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@Blast545 Blast545 requested a review from brawner May 1, 2020 21:35
Copy link
Copy Markdown
Contributor

@brawner brawner left a comment

Choose a reason for hiding this comment

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

Minor nits, but otherwise looks good to me.

@ahcorde
Copy link
Copy Markdown
Contributor

ahcorde commented May 4, 2020

add the QD to the README.md

Blast545 added 2 commits May 4, 2020 18:51
Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@EduPonz
Copy link
Copy Markdown

EduPonz commented Jan 19, 2021

Due to Fast DDS reaching QL1 (eProsima/Fast-DDS#1610), I think this PR can be closed

@Blast545 Blast545 closed this Jan 21, 2021
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.

4 participants