-
Notifications
You must be signed in to change notification settings - Fork 15
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
feat: enhance chart features, add daemonset, add linting ci #24
Conversation
Thanks for contributing! |
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 your contribution. I think your changes greatly improve the docker-hub-rate-limit-exporter. I especially like the inclusion of helm tests - I didn't know this tool before! Great work!
However, there are a few issues with your changes that need to be addressed before merging. If you need help with fixing the issues because of time constraints please let me know. I really would love for this to be merged soon. :-)
charts/docker-hub-rate-limit-exporter-chart/templates/deployment.yaml
Outdated
Show resolved
Hide resolved
charts/docker-hub-rate-limit-exporter-chart/templates/deployment.yaml
Outdated
Show resolved
Hide resolved
charts/docker-hub-rate-limit-exporter-chart/templates/deployment.yaml
Outdated
Show resolved
Hide resolved
charts/docker-hub-rate-limit-exporter-chart/templates/deployment.yaml
Outdated
Show resolved
Hide resolved
charts/docker-hub-rate-limit-exporter-chart/templates/servicemonitor.yaml
Show resolved
Hide resolved
add: list of deployments
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 again for your quick response in fixing the review comments from earlier :-)
I think we can merge this MR once the tests are green and once we revert the changes regarding multiple deployments / multi-account-feature, as elaborated on in the comment.
If you want to help with the multi-accoount feature, please open another pull request so we can track (and merge) the progress separately.
Thanks again for your contribution :-)
charts/docker-hub-rate-limit-exporter-chart/ci/all-enabled-values.yaml
Outdated
Show resolved
Hide resolved
Thanks again for contributing :-) |
No problem, you can mention my github username 🙂 |
No description provided.