search

ros2 / rosidl_typesupport

Packages which provide the typesupport for ROS messages and services
Apache License 2.0
13 stars 34 forks source link

Update Quality Declarations to level 3. #77

Closed clalancette closed 3 years ago

clalancette commented 4 years ago

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

clalancette commented 3 years ago

I've now fixed most of the thing from review. However, there are still some dependencies here that do not have QUALITY_DECLARATIONS at all, so I think this needs to be on hold until that is resolved.

brawner commented 3 years ago

rosidl_typesupport_connext_c and rosidl_typesupport_introspection_c were purposely left off of the quality levels effort. You might need to ask @chapulina or @wjwwood for why that was and just add that reasoning to the Quality Declaration.

I think it's fine to merge, since the push to QL 3 doesn't require its dependencies to be QL 3.

wjwwood commented 3 years ago

They were left out, I assume, because they are not affecting the runtime quality of the package. But I don't know that for sure without digging into it again. That's part of what needs to be considered and captured in the QD for this package. @chapulina and I had a file where there were comments about why things were included/excluded at various times, but it should be possible to reason about it again and come to a similar conclusion.

clalancette commented 3 years ago

They were left out, I assume, because they are not affecting the runtime quality of the package. But I don't know that for sure without digging into it again. That's part of what needs to be considered and captured in the QD for this package. @chapulina and I had a file where there were comments about why things were included/excluded at various times, but it should be possible to reason about it again and come to a similar conclusion.

Yeah, you are right about that. Looking at it, those two packages are excluded because they are there as a workaround for a bloom limitation. As such, there are no direct runtime dependencies in this package on them. So I've now removed them from the QUALITY_DECLARATION.md completely.

As such, I think this is ready to go. I have approval from @brawner ; I would prefer to get approval from @ahcorde (original reviewer) before merging. Thanks!

clalancette commented 3 years ago

OK, looks like this one has 2 approvals, so I'll go ahead and merge. Thanks everyone!