Comments (10)
If you need an a helping hand than I could support as repository admin. From my understanding repository admins should not approve PRs for charts unless they are maintainer of the chart or one of the maintainers of a chart explicitly asks for it.
@scottrigby You did a great job in setting all this up and driving discussions. At least I would appreciate if you could continue helping as admin.
from helm-charts.
Cool, thanks for the explanation @scottrigby , all makes sense now
from helm-charts.
Hey @timm088 π thanks for asking! This issue needs clarification and closure.
There was a little confusion above - the CODEOWNERS file already exists, and added your ownership for the consul exporter chart:
helm-charts/.github/CODEOWNERS
Line 13 in c30ca33
That means you should already get a review request whenever someone opens a PR with any changes on it π there should be nothing for there to do there.
@torstenwalter 's suggestion above was for all the chats in this repo to have at least two maintainers (there's another issue for that: #21). @gkarthiks made a PR to add him as a co-maintainer of your chart in #50. It's up to you though as a (currently only) maintainer of that chart to approve his request in that PR or not.
This issue was me asking all the current prom chart maintainers opinion on how repo administration should work (if people would like to rotate, vote on people who have shown they want to and are able to do it, etc). @torstenwalter volunteered earlier, and we worked together to set up a few other community chart repos (grafana and jenkins), so I added him as a repo admin. We now have a PROCESSES doc file for this repo, so you can all make PRs against that and discuss any additional process ideas and needs you have there.
For now I think this issue is done because it's not just me as admin anymore, so I'm going to close it. Let's all continue in the other issues π
from helm-charts.
I guess everyone of the chart maintainers can also review non app specific stuff such as image tag updates and so on, so the abiltiy to "merge PRs for other charts they don't maintain" makes sense to me as this would fasten up review processes.
from helm-charts.
@torstenwalter Also agree charts with a single maintainer should ideally have at least two. Hopefully people can work that out in #21. Do you want to move that part of your comment there?
Just moved it there #21 (comment)
from helm-charts.
I just opened PR #44 which adds CODEOWNERS according to the list above.
@scottrigby I saw that you defined them in #1. This splits that up so that we could merge it independent of that.
I also adjusted the syntax to /chart/<name-of-chart> @maintainer
.
It matches any files in the chart directory at the root of the repository and any of its subdirectories.
Without the leading / it would also match directories found somewhere else. It's unlikely that those names would be used, but it does not harm to do it this way.
Also added:
## changes to CODEOWNERS should be reviewed by repository admins
/CODEOWNERS @helm-charts-admins
The intention is that changes to the CODEOWNERS file also needs to be reviewed by one of the repository admins. One could argue, that it could delay the process of adding maintainers. In case one maintainer of a chart is added to another chart that is true! In case an external person is added that's not really the case as one of the repository administrators would need to grant him/her write permissions to the repository anyhow.
I also added a draft for PROCESSES.md
, which at the moment just outlines how to add maintainers. We could use that file for further process documentation if needed.
When introducing CODEOWNERS we will have an issue with charts which only have one maintainers. If that maintainer make a change then he is not able to approve it. Repository admins would need to use their "superpower" to override the required review and can merge it. That might be ok for the start, but I would suggest that we try to add at least a second maintainer there.
Affected charts are:
- charts/prometheus-consul-exporter @timm088
- charts/prometheus-couchdb-exporter @gkarthiks
- charts/prometheus-mongodb-exporter @steven-sheehy
- charts/prometheus-postgres-exporter @gianrubio
- charts/prometheus-rabbitmq-exporter @Juanchimienti
- charts/prometheus-redis-exporter @acondrat
- charts/prometheus-snmp-exporter @Miouge1
- charts/prometheus-to-sd @acondrat
Would be great if we could fined volunteers here or even better if the chart maintainers could try to motivate people who already contributed to the chart to become maintainer.
from helm-charts.
@torstenwalter I updated your #44 PR title to be just about the PROCESSES documentation (which is great!), because /.github/CODEOWNERS already exists (I added it there in #12, for tidiness).
I also made a review comment to include some of the processes in the chat above, like what you just mentioned about admin and chart maintainer roles relating to PRs.
from helm-charts.
@torstenwalter Also agree charts with a single maintainer should ideally have at least two. Hopefully people can work that out in #21. Do you want to move that part of your comment there?
from helm-charts.
@torstenwalter youβre now a member of the @prometheus-community/helm-charts-admins team. So we have at least a few people to help with this side of things for now.
Let's all make sure processes like team membership (criteria, responsibilities and limitations) explicit in the PROCESSES doc being added in #44.
Once that's merged, we can probably close this issue and make further amendments to the PROCESSES doc in follow-up PRs, and discuss other questions or ideas that come up in new issues.
from helm-charts.
@scottrigby @torstenwalter not entirely sure whats being requested?
Haven't modified the consul-exporter chart in a while, but would be happy to assist where it makes sense.
Is the request here to try to gather support from existing contributors on that chart, to become CODEOWNERs?
Or , nothing to do and just an FYI?
from helm-charts.
Related Issues (20)
- [kube-prometheus-stack] grafana okta sso setup
- [Prometheus-adapter] Does Prometheus-adapter support scraping custom metrics for windows pods to implement HPA.
- [kube-prometheus-stack] How to set kube-prometheus-stack Grafana sidecar containers resource limit? HOT 4
- [prometheus-stackdriver-exporter] Deployment metadata missing annotations
- [kube-state-metrics] DaemonSet option missing
- [kube-prometheus-stack] unknown field "automountServiceAccountToken" HOT 1
- [kube-prometheus-stack] override image repository from helm command line (--set) not working HOT 2
- [kube-prometheus-stack] no specific registry key of .Values.prometheusOperator.admissionWebhooks or .patch is respected if global.registry is set
- [prometheus-elasticsearch-exporter] image.pullSecret not taken into account HOT 2
- [kube-prometheus-stack] Networking NO DATA HOT 4
- [prometheus-windows-exporter] Readiness probe always returning 404 HOT 1
- [prometheus] How to set podLabels for the remaining pods (alertmanager, node-exporter, pushgateway, etc.) HOT 1
- [kube-prometheus-stack] Wrong Grafana default E-Mail
- [kube-prometheus-stack] retentionSize is not taken into effect HOT 1
- [prometheus-kube-stack] gcfsd filesystem has less than 5% space left
- [kube-prometheus-stack] Node Exporter Job name changed after upgrade fro 61.2.0 to 62.1.0 HOT 4
- [kube-prometheus-stack] Issue with Metric to fetch k8's cluster nodes memory usage HOT 1
- [kube-prometheus-stack] Issue with the Prometheus upgrade from 61.8.0 to 62.2.1 HOT 4
- [kube-prometheus-stack] grafana won't display node-exporter metrics on minikube HOT 1
- [kube-prometheus-stack] Missing flexibility support for single/multi cluster alert from kubernetes-mixin
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
π Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. πππ
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google β€οΈ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from helm-charts.