-
Notifications
You must be signed in to change notification settings - Fork 102
[#666] Add Quality Declaration #667
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.
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
base: main
Are you sure you want to change the base?
[#666] Add Quality Declaration #667
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a quality declaration for the iceoryx2 repository by introducing a new quality levels document and updating the README to reference it.
- Added a "Quality Levels" section with an overview table in the README.md.
- Created a new file (QUALITY_LEVELS.md) that details the quality levels based on ROS standards.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| README.md | Added a link and a new section explaining quality levels with a comparison table. |
| QUALITY_LEVELS.md | New file that describes each quality level and its requirements. |
Comments suppressed due to low confidence (2)
README.md:333
- [nitpick] Consider using the plural form 'Quality Levels' in the link text for consistency with the section title and overall documentation.
[Quality level](./QUALITY_LEVELS.md) are 5 to 1+, where 1+ is the highest level.
QUALITY_LEVELS.md:1
- [nitpick] Consider capitalizing 'levels' to 'Levels' to match the styling used in the README and maintain consistency across documentation.
# Quality levels
|
|
||
| * Derived from [ROS quality level 4](https://www.ros.org/reps/rep-2004.html#quality-level-4) | ||
| * Basic unit tests are required | ||
| * Builds and runs on Windows, macOS, Linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would state that it runs on every supported platform and then we link to the list of supported platforms.
|
|
||
| * Derived from [ROS quality level 1](https://www.ros.org/reps/rep-2004.html#quality-level-1) | ||
| * Version policy for stable API and ABI required | ||
| * [ASPICE](https://beza1e1.tuxen.de/aspice.html) SWE.6 tests available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The provided link does not state anything about SWE.6 tests, what are they, how are they defined. This needs to be at least linked.
| ## Quality level 1 | ||
|
|
||
| * Derived from [ROS quality level 1](https://www.ros.org/reps/rep-2004.html#quality-level-1) | ||
| * Version policy for stable API and ABI required |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also link our version policy (semver) so that it is clear what is meant by that.
| * [ASPICE](https://beza1e1.tuxen.de/aspice.html) SWE.6 tests available | ||
| * Performance tests and regression policy required | ||
| * Static code analysis warnings addressed | ||
| * Enforcing the code style is required |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quality level 5. This is a minimum.
For this and the static code analysis we should define what we mean by that. In this case, follow clang-format or rustfmt.
| * Version policy for stable API and ABI required | ||
| * [ASPICE](https://beza1e1.tuxen.de/aspice.html) SWE.6 tests available | ||
| * Performance tests and regression policy required | ||
| * Static code analysis warnings addressed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quality level 5 but we can restrict this to: Rust (clippy, no compiler warnings and miri), C++/C (clang-tidy, address/undefined behavior/thread sanitizer, no compiler warnings)
|
|
||
| ## Quality level 2 | ||
|
|
||
| This quality level is meant for all targets that need tier 1 support in ROS 2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove this statement and would in every quality level maybe add: conforms with ros2 quality level X.
Also we could state then things like: conforms with ISO 26262 ASIL-D (later) and add also other standards.
Those things have an actual meaning.
| * Derived from [ROS quality level 1](https://www.ros.org/reps/rep-2004.html#quality-level-1) | ||
| * Version policy for stable API and ABI required | ||
| * [ASPICE](https://beza1e1.tuxen.de/aspice.html) SWE.6 tests available | ||
| * Performance tests and regression policy required |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is a regression policy.
| * Performance tests and regression policy required | ||
| * Static code analysis warnings addressed | ||
| * Enforcing the code style is required | ||
| * Unit tests have full statement and branch coverage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't full statement coverage a subset of branch coverage? So branch coverage would suffice?!
|
|
||
| ## Quality level 1+ | ||
|
|
||
| This quality level goes beyond the ROS quality levels and contains extensions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this line. I would not reference ROS2 here. They are, in my opinion, not a valid reference for code quality.
|
|
||
| | Component | Current Level | Target Level for Tier 1 Platforms | Target Level for Tier 2 Platforms | | ||
| |-----------------------|:-------------:|:---------------------------------:|:---------------------------------:| | ||
| | iceoryx2 | 2 | 1+ | 1 | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use nowhere quality level 3. Should we then merge quality level 2 with 3? Also 1 and 1+ are a bit weird.
I would make this its own normal quality level.
So it would be:
- 1+ -> 1
- 1 -> 2
- 2 and 3 -> 3
- 4 -> 4
- 5 -> 5
Notes for Reviewer
This PR adds a quality declaration similar to the one from iceoryx classic.
Pre-Review Checklist for the PR Author
Convert to draft)SPDX-License-Identifier: Apache-2.0 OR MITiox2-123-introduce-posix-ipc-example)[#123] Add posix ipc example)task-list-completed)Checklist for the PR Reviewer
Post-review Checklist for the PR Author
References
Closes #