Skip to content
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

Update QD to Quality Level 1 #66

Merged
merged 3 commits into from
Aug 25, 2020
Merged

Conversation

brawner
Copy link
Contributor

@brawner brawner commented Aug 25, 2020

Signed-off-by: Stephen Brawner brawner@gmail.com

Signed-off-by: Stephen Brawner <brawner@gmail.com>
Copy link
Contributor

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

lgtm

During my review I read the whole document, and though out of the scope of this pr, I found to issues. First the link in this line is 404 for me:

All the functionality of the declared API in this package is covered in its unit tests. Currently it has a line coverage of 99%.

Also, this line doesn't make sense to me:

  • achieving and maintaining a reasonable branch line coverage (90-100%)

It should be branch or line coverage or just branch coverage or just line coverage, maybe?

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Copy link
Contributor Author

brawner commented Aug 25, 2020

I agree that this PR would be suitable for fixing up any remaining issues. It will get less frequent updates now that it's been bumped to QL 1. I updated the coverage link and updated that line to read branch or line coverage.

Copy link
Contributor

@Blast545 Blast545 left a comment

Choose a reason for hiding this comment

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

Took a look to the whole document, LGTM after a minor change

ament_index_cpp/QUALITY_DECLARATION.md Show resolved Hide resolved
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@Blast545 Blast545 merged commit c9778d8 into ament:master Aug 25, 2020
@chapulina chapulina mentioned this pull request Nov 26, 2020
22 tasks
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.

None yet

3 participants