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

Removal of redundant values #103

Merged
merged 4 commits into from
Jan 21, 2022
Merged

Removal of redundant values #103

merged 4 commits into from
Jan 21, 2022

Conversation

AndersBennedsgaard
Copy link
Contributor

In values.yaml, monitoring.namespace: cattle-dashboards leads the developer to think that this namespace is where Grafana should be located on the cluster. However from charts/policy-reporter/charts/monitoring/templates/_helpers.tpl, it should actually be monitoring.grafana.namespace.

This PR gives a description that monitoring.namespace should not be used and only remains due to backwards compatability (#92), and removes monitoring.serviceMonitor defaults which should be left to the monitoring subchart.

@fjogeleit
Copy link
Member

fjogeleit commented Jan 19, 2022

Hey, thanks for your contribution.

You're right, I missed this in the cleanup. monitoring.namespace can be removed because it was removed in v2.0 of the Helm Chart in the monitoring subchart.

Can you also please recommit with the -s to sign the commit and fulfill the DCO?

git reset --soft HEAD~1
git add .
git commit -m "..." -s
git push origin main -f

Signed-off-by: Anders Bennedsgaard <abbennedsgaard@gmail.com>
Signed-off-by: Anders Bennedsgaard <abbennedsgaard@gmail.com>
Signed-off-by: Anders Bennedsgaard <abbennedsgaard@gmail.com>
Signed-off-by: Anders Bennedsgaard <abbennedsgaard@gmail.com>
@AndersBennedsgaard
Copy link
Contributor Author

Sorry, I'm not very used to working on OSS, but hopefully it works now 😄

@fjogeleit
Copy link
Member

fjogeleit commented Jan 20, 2022

Yes, looks good. If the linting fails because of the unchanged version, that’s no problem. I will prepare it today. Thank you

@fjogeleit fjogeleit merged commit 690dbb0 into kyverno:main Jan 21, 2022
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.

2 participants