rewrite of the package quality categories section of the developer guide

[wjwwood]

I'm opening this pull request to start a discussion about the proposed changes to the (pre-existing) package quality categories with the aim to make use of them in the near future.

The end goal would be to move most of this into a REP, and then replace this section with the prescriptions on how to achieve the categories for ROS core packages.

https://ros2-documentation-pr-460.herokuapp.com/Contributing/Developer-Guide.html#package-quality-categories

[chapulina]

This is really thorough, William! A great compilation of good software practices!

One thing I think could make it easier to see the differences between the levels would be to put the requirements side-by-side, maybe in a table, but I'm not sure how it could fit nicely.

[dirk-thomas]

In parallel to this document it would be great to maintain a list of missing "features" which are necessary to enable the proposed required checks (e.g. coverage collection or performance tests on PRs).

[clalancette]

Left a few comments on things I saw.

[ablasdel]

Thanks for this detailed and well thought out writeup on ROS2 quality William! Some questions and comments.

[thomas-moulard]

This is really awesome! Adding more ideas, not sure if they are all in scope or not.

[thomas-moulard]

Two other things to consider:

• Release Management
  • Software will be released in Bloom regularly
  • A release cadence / policy is defined
• Security / Vulnerability Management. If a vulnerability is detected in this software, a new version fixing the bug will be released with an SLA of X. ?

[wjwwood]

Software will be released in Bloom regularly

I don't see why this is needed to assess the quality of the package...

A release cadence / policy is defined

I'm not sure this makes sense for most packages, and while useful I'm not sure it should be required.

Security / Vulnerability Management. If a vulnerability is detected in this software, a new version fixing the bug will be released with an SLA of X. ?

For our current packages, I think the SLA is ASAP. I don't think there will be a lot of difference here or a guarantee better than ASAP right now, so I wouldn't require it.

Furthermore, I don't know how I would right that into a requirement or what fulfilling that requirement would look like. I think a lot more discussion needs to happen here to have this added.

---

For each of these proposed requirements, I think who ever is interested in them, should push discussion beyond just with me and propose the wording as a discussion point.

[wjwwood]

Bump here.

I'm open to adding stuff about the release management and security, but I just summarized above why I avoided it in the first pass.

If you guys agree for now with me, I can resolve this comment thread, otherwise please let me know why you think it should be incorporated. And in the case of security please suggest what kind of requirements we could reasonably make (or how we could word it).

[thomas-moulard]

I don't want to block the PR on this, but don't you think that a high quality package has a clear story for how end-users will get to use it? I am worried that this doc is about "writing good C++/Python/..." and misses the larger point. Ditto for the security vulnerabilities, I understand we don't want to commit to a number of days, but without some policy around vulnerability management, it will make the software harder to adopt for a company.

[wjwwood]

a clear story for how end-users will get to use it

For me it's really specific to the ecosystem and for many software packages the people packaging the software are completely separate from the developers that decide when and how to make releases (provision new versions). The version policy covers how version numbers increase, and that should be enough to let packagers decide what versions to include on what platforms/distributions.

I can make a ROS specific requirement that is it must be released into a ROS distribution, but the idea of saying "regularly" or some other qualifier seems unnecessary. A package may be released only once per distribution and be completely fine, for example...

[wjwwood]

Ditto for the security vulnerabilities, I understand we don't want to commit to a number of days, but without some policy around vulnerability management, it will make the software harder to adopt for a company.

I actually think this is worth having in, but I don't know what to make the requirement. We could say "must have a security policy", but I feel like (at least for level 1) there should be some qualifiers on that policy. Put another way, is it ok for a level 1 package to have a policy for security that there is no process to handle vulnerabilities and everything is best effort?

[wjwwood]

We'll follow up on this after merging.

[clalancette]

Two other things came to my mind when mulling this over yesterday:

1. We may need a special category for hardware drivers, or at least have a discussion about them. In general, it's pretty hard to get high code coverage testing on hardware drivers; you either have to have a hardware-in-the-loop testing system (expensive both in time and money), or an "emulator" of the hardware to test against in software (expensive in time to write, and may just confirm existing bugs in the driver).
2. I think we should be careful about adding too many requirements to the Level 1 category. In particular, if we make the requirements too onerous, then essentially no packages will be able to be in the category. For instance, I don't think any of the ROS 2 repositories (core or otherwise) will be in the current definition of Level 1. We should obviously strive to do better, but I think we need to strike a balance between practicality and improving the software.

[wjwwood]

One thing I think could make it easier to see the differences between the levels would be to put the requirements side-by-side, maybe in a table, but I'm not sure how it could fit nicely.

I agree, I'll work on that.

[wjwwood]

We may need a special category for hardware drivers, or at least have a discussion about them. In general, it's pretty hard to get high code coverage testing on hardware drivers; you either have to have a hardware-in-the-loop testing system (expensive both in time and money), or an "emulator" of the hardware to test against in software (expensive in time to write, and may just confirm existing bugs in the driver).

Or, you could write your software in a way that you could mock the interface to the hardware so you can unit and integration test without hardware in the loop. I suppose that might be what you mean when you say "emulator", but I'm thinking narrower, e.g. if you have a function to deserialize binary data from a sensor, then you should be able to say what happens when "I get unexpected data", "get too little data", "get too much data", etc...

Also, you can always set a lower coverage number and justify it. Users (and peer reviewers) will read your justification and decide for themselves if it is reasonable.

Personally, if a package said "I don't test this ~20% chunk of code because it's hard" I would not consider it to be high quality, hardware or not...

I think we should be careful about adding too many requirements to the Level 1 category. In particular, if we make the requirements too onerous, then essentially no packages will be able to be in the category.

I think it should include anything we think is reasonable, because there is no higher standard, we have level 2 for in between stuff. I think having too many packages in level 1 is also bad.

I think the criteria for the categories should be driven by what users of those packages want to see for the considered use case (e.g. production versus tools, etc.). Not by how many packages get in and out of the category.

I'd prefer to see more graduations of categories before watering down the highest category.

For instance, I don't think any of the ROS 2 repositories (core or otherwise) will be in the current definition of Level 1.

I would expect none of them reach level 1 and most don't reach level 2, but that means we have work to do, not that our categories are too strict (imo).

Changing the categories based on the result of how the current packages fall into them seems backwards to me.

We should obviously strive to do better, but I think we need to strike a balance between practicality and improving the software.

I think that practicality should come from wisely deciding which packages should strive for which categories and trying to minimize the amount of code that goes toward level 1, not by adjusting the requirements of the categories.

I think we should try to set good standards not include as many of our packages in the "top" level as easily as possible.

---

I expect that most packages will land in level 2 or 3 and that will be fine.

Aside from reaching a version >= 1.0.0, collecting coverage (not even enforcing it), documenting all the features, and having a "quality declaration", I think most of the core packages are close to level 2 already. Only the most important packages should strive to get to level 1, which will involve a lot of work.

[wjwwood]

I think I've caught up on feedback so far. Thanks for all the initial feedback!

I'll propose we merge this after the current feedback is iterated on, and then I can post start the migration to a REP (at least to kick off discussion).

I can also then come up with a proposed list for packages that we should aim for level 1.

[jacobperron]

Looks good! My only wish is for a table of quality levels versus requirement categories. I think it would make for a nice summary of the differences in levels. But, maybe it's not feasible to squeeze everything in one table.

[wjwwood]

My only wish is for a table of quality levels versus requirement categories. I think it would make for a nice summary of the differences in levels. But, maybe it's not feasible to squeeze everything in one table.

Still working on that. I think I can make work.

[wjwwood]

I bumped a few comment threads for more feedback, and resolved a few others. If you felt like you had unresolved outstanding comments, please have a look and feel free to unresolve them or comment if you think they should continue to be discussed.

[thomas-moulard]

This LGTM, I have some concerns left around release management, updating dependencies and linting, but those are pretty minor.

[chapulina]

Don't want to block on the linters, will leave it up to you, @wjwwood .

Must have style guidelines and enforce them.

This, however, does make sense to me. I'll add it.

Also I think you forgot to add the bit about style.

[ablasdel]

Thanks for the changes added.
I think this is a solid way to think about quality in the ROS 2 ecosystem.

[wjwwood]

Ok, the only thing missing now is something about handling security vulnerabilities. So I'll fixup CI and then plan to merge this.

We can continue the discussion with more people involved and including the security stuff when I migrate this to a REP in the next steps.