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

[#48] Adding audit scripts #49

Merged
merged 5 commits into from
Jan 5, 2024

Conversation

hydroid7
Copy link
Contributor

@hydroid7 hydroid7 commented Dec 16, 2023

Notes for Reviewer

Pre-Review Checklist for the PR Author

  1. Add sensible notes for the reviewer
  2. PR title is short, expressive and meaningful
  3. Relevant issues are linked
  4. Every source code file has a copyright header with SPDX-License-Identifier: Apache-2.0 OR MIT
  5. Branch follows the naming format (iox2-123-introduce-posix-ipc-example)
  6. Commits messages are according to this guideline
  7. Tests follow the best practice for testing
  8. Changelog updated in the unreleased section including API breaking changes
  9. Assign PR to reviewer
  10. All checks have passed (except task-list-completed)

Checklist for the PR Reviewer

  • Commits are properly organized and messages are according to the guideline
  • Unit tests have been written for new behavior
  • Public API is documented
  • PR title describes the changes

Post-review Checklist for the PR Author

  1. All open points are addressed and tracked via issues

References

Use either 'Closes #123' or 'Relates to #123' to reference the corresponding issue.

Closes #ISSUE-NUMBER

@elfenpiff elfenpiff marked this pull request as ready for review December 16, 2023 23:59
Copy link
Contributor

@elfenpiff elfenpiff left a comment

Choose a reason for hiding this comment

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

Could you please add this to doc/release-notes/iceoryx2-unreleased.md in the workflow section.

The entry would look like this here:

* add `cargo audit` for security vulnerability checking in dependencies [#49](https://github.com/eclipse-iceoryx/iceoryx2/issues/49)

All the other examples still have the old repo as link, when you edit the file could you please update these links as well - thanks!

When the changelog is updated we can merge this. @elBoberido you know cirrus best, any objections?

.cirrus.yml Outdated Show resolved Hide resolved
@hydroid7
Copy link
Contributor Author

Could you please add this to doc/release-notes/iceoryx2-unreleased.md in the workflow section.

The entry would look like this here:

* add `cargo audit` for security vulnerability checking in dependencies [#49](https://github.com/eclipse-iceoryx/iceoryx2/issues/49)

All the other examples still have the old repo as link, when you edit the file could you please update these links as well - thanks!

When the changelog is updated we can merge this. @elBoberido you know cirrus best, any objections?

Also added for this PR.

Copy link
Member

@elBoberido elBoberido left a comment

Choose a reason for hiding this comment

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

Looks good from my point of view. We still might need to add a nightly build but that's for another PR not this one

@@ -18,7 +18,7 @@

### Workflow

* Example [#1](https://github.com/larry-robotics/iceoryx2/issues/1)
* add `cargo audit` for security vulnerability checking in dependencies [#49](https://github.com/eclipse-iceoryx/iceoryx2/issues/49)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* add `cargo audit` for security vulnerability checking in dependencies [#49](https://github.com/eclipse-iceoryx/iceoryx2/issues/49)
* add `cargo audit` for security vulnerability checking in dependencies [#48](https://github.com/eclipse-iceoryx/iceoryx2/issues/48)

Please always link to the actual issue instead of the PR :)

Copy link
Contributor

Choose a reason for hiding this comment

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

@hydroid7 whoopsie my mistake I mixed up the PR number and the issue number

Copy link
Contributor

@elfenpiff elfenpiff left a comment

Choose a reason for hiding this comment

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

Same here, as soon as the issue number in the release notes is corrected we can merge this.

Copy link
Member

@elBoberido elBoberido left a comment

Choose a reason for hiding this comment

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

Could you please add [#48] to the Fixes release notes commit message.

Oh, and for future commits, please write the commit messages in imperative so Fix bug and not Fixes bug or Fixing bug. This is just to prevent proliferation and to have a uniform style in the commit messages.

Thanks for being so patient with us :)

@hydroid7 hydroid7 force-pushed the iox-2-48-cargo-audit-ci branch 2 times, most recently from 8558027 to 43fda38 Compare December 28, 2023 19:37
elBoberido
elBoberido previously approved these changes Dec 28, 2023
Copy link
Member

@elBoberido elBoberido left a comment

Choose a reason for hiding this comment

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

Oh no, now there is a merge conflict in the release notes. You have to rebase before it can be merged

Copy link

codecov bot commented Jan 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (87d0d8f) 78.00% compared to head (077cbf0) 78.01%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #49      +/-   ##
==========================================
+ Coverage   78.00%   78.01%   +0.01%     
==========================================
  Files         172      172              
  Lines       18782    18782              
==========================================
+ Hits        14650    14653       +3     
+ Misses       4132     4129       -3     

see 1 file with indirect coverage changes

@elfenpiff elfenpiff merged commit 8afd996 into eclipse-iceoryx:main Jan 5, 2024
21 checks passed
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.

3 participants