-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[extension/healthcheckv2extension] Refactor internal/status to pkg/status #35648
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.
Thanks for taking on this refactor. I made a PR to this PR with a few additional updates that will be needed. In addition to those, this should have a changelog entry. Run make chlog-new
to generate one and fill in the details.
Versons, builder config, and metadata
Co-authored-by: Matthew Wear <matthew.wear@gmail.com>
@mwear Thanks for the cleanup PR. I have merged your PR and updated the |
@mwear Forgot about the changelog. I have added that as well. |
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.
LGTM, thanks @blakerouse!
cc: @bacherfl, @evan-bradley re: #35488.
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.
Thank you @blakerouse!
Overall this looks good to me, I just have comments on a few supplementary details.
@evan-bradley I have update the requested items. But running
|
I'm not sure why that doesn't work for a subdirectory, but you can run |
@mwear I did do that. I didn't see it generate or do anything. Maybe there is nothing to generate? |
It looks like there are two more changes needed: Add a doc.go file with the following contents:
Update metadata.yml to include a type, you can just copy this:
I found that if you cd into the pkg/status directory you can run |
@mwear @evan-bradley Thanks for the help on getting this ready. I think its ready now as the generation logic worked this time and filled in the correct information in the README. |
The currently failing CI check says to run |
@mwear Okay. maybe now its ready. 😄 |
Thank you @blakerouse! Your patience while working through the process of setting up a new module is appreciated. |
No problem! Glad to help. |
Description
Refactors the
extension/healthcheckv2extension/internal/status
intopkg/status
.This exposes the aggregator to be used by other extensions to gather component status information using the
extension.StatusWatcher
.Link to tracking issue
Closes #34692
Testing
Being it was a refactor and all the same tests provided coverage, no additional testing was added.
Documentation
Added a
README.md
to thepkg/status
to provide information on where this package can be used.