containerd / nri

Node Resource Interface
Apache License 2.0
257 stars 65 forks source link

Updated the OCI Hook Injector README to resovle broken links to the p… #34

Closed swgriffith closed 1 year ago

swgriffith commented 1 year ago

Resolves issue #33

Updates the links to podman documentation to reference the proper sha1 and adds additional details to help guide people in testing the hook-injector plugin, including relevant path details.

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: +0.16 :tada:

Comparison is base (2a8b655) 63.83% compared to head (57cfa26) 64.00%.

:exclamation: Current head 57cfa26 differs from pull request most recent head c783fc7. Consider uploading reports for the commit c783fc7 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #34 +/- ## ========================================== + Coverage 63.83% 64.00% +0.16% ========================================== Files 9 9 Lines 1800 1800 ========================================== + Hits 1149 1152 +3 + Misses 500 497 -3 Partials 151 151 ``` [see 1 file with indirect coverage changes](https://codecov.io/gh/containerd/nri/pull/34/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd) Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

kad commented 1 year ago

Maybe it would be more sense to point to top of the tree like https://github.com/containers/common/tree/main/pkg/hooks and https://github.com/containers/common/blob/main/pkg/hooks/docs/oci-hooks.5.md instead of some older commit id in podman repo?

klihub commented 1 year ago

Maybe it would be more sense to point to top of the tree like https://github.com/containers/common/tree/main/pkg/hooks and https://github.com/containers/common/blob/main/pkg/hooks/docs/oci-hooks.5.md instead of some older commit id in podman repo?

We deliberately chose to point to the exact version we use in the plugin. That is then a different story that we should really file another PR to update the plugin to use a recent version, and then (remember to) update the link in the same PR as well.

swgriffith commented 1 year ago

@klihub and @kad - It sounds like we're leaving it as is. Let me know if you need me to change the URLs.

kad commented 1 year ago

I'm ok with @klihub opinion, so lgtm.