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.

Sign up for GitHub

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

Jump to bottom

[ci] Update codeowners script #5212

Merged
merged 2 commits into from
Jan 20, 2025
Merged

[ci] Update codeowners script #5212

merged 2 commits into from
Jan 20, 2025

Conversation

SuperQ
Copy link
Contributor

@SuperQ SuperQ commented Jan 19, 2025 •
edited
Loading

What this PR does / why we need it

  • Fixup issue link to point to correct location.
  • Remove @jkroepke from manual listing now that they are a member of helm-chart-admins.
  • Refactor scripts/check-codeowners.sh to simplify it.

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

  • fixes #

Special notes for your reviewer

Checklist

  • DCO signed
  • Chart Version bumped
  • Title of the PR starts with chart name (e.g. [prometheus-couchdb-exporter])

@SuperQ SuperQ requested a review from a team as a code owner January 19, 2025 16:41
@SuperQ SuperQ requested a review from jkroepke January 19, 2025 16:42
@SuperQ SuperQ force-pushed the superq/update_script branch 2 times, most recently from 1b094e0 to c1ff9ab Compare January 19, 2025 17:15
@SuperQ SuperQ force-pushed the superq/update_script branch 2 times, most recently from fc5bfab to d46f86e Compare January 19, 2025 17:20
@SuperQ
Copy link
Contributor Author

SuperQ commented Jan 19, 2025

I don't get why the sort isn't working.

@SuperQ SuperQ force-pushed the superq/update_script branch from d46f86e to bc1fe3c Compare January 19, 2025 17:43
@jkroepke
Copy link
Member

jkroepke commented Jan 19, 2025 •
edited
Loading

Sort looks good (based on the tee output on actions:

Details 
/charts/alertmanager-snmp-notifier/ @maxwo
/charts/alertmanager/ @monotek @naseemkullah
/charts/jiralert/ @jkroepke @zanhsieh
/charts/kube-prometheus-stack/ @andrewgkew @gianrubio @gkarthiks @GMartinez-Sisti @jkroepke @QuentinBisson @scottrigby @Xtigyro
/charts/kube-state-metrics/ @dotdc @mrueg @tariq[18](https://github.com/prometheus-community/helm-charts/actions/runs/12855797074/job/35841742982?pr=5212#step:4:19)90
/charts/prom-label-proxy/ @jkroepke
/charts/prometheus-adapter/ @hectorj2f @mattiasgees @steven-sheehy
/charts/prometheus-blackbox-exporter/ @desaintmartin @gianrubio @monotek @rsotnychenko
/charts/prometheus-cloudwatch-exporter/ @asherf @gianrubio @torstenwalter
/charts/prometheus-conntrack-stats-exporter/ @monotek
/charts/prometheus-consul-exporter/ @gkarthiks @timm088
/charts/prometheus-couchdb-exporter/ @gkarthiks
/charts/prometheus-druid-exporter/ @iamabhishek-dubey @sandy724
/charts/prometheus-elasticsearch-exporter/ @desaintmartin @svenmueller @zeritti
/charts/prometheus-fastly-exporter/ @arslanbekov
/charts/prometheus-ipmi-exporter/ @lexfrei
/charts/prometheus-json-exporter/ @schmiddim @xiu @zanhsieh
/charts/prometheus-kafka-exporter/ @gkarthiks @golgoth31 @zeritti
/charts/prometheus-memcached-exporter/ @rsicart
/charts/prometheus-modbus-exporter/ @openenergyprojects
/charts/prometheus-mongodb-exporter/ @steven-sheehy @zeritti
/charts/prometheus-mysql-exporter/ @juanchimienti @monotek
/charts/prometheus-nats-exporter/ @caarlos0 @okgolove
/charts/prometheus-nginx-exporter/ @nlamirault @zeritti
/charts/prometheus-node-exporter/ @gianrubio @zanhsieh @zeritti
/charts/prometheus-opencost-exporter/ @mattray
/charts/prometheus-operator-admission-webhook/ @zeritti
/charts/prometheus-operator-crds/ @dacamposol @desaintmartin @jkroepke @QuentinBisson
/charts/prometheus-pgbouncer-exporter/ @stewartshea @zeritti
/charts/prometheus-pingdom-exporter/ @monotek @rpahli
/charts/prometheus-pingmesh-exporter/ @dongjiang[19](https://github.com/prometheus-community/helm-charts/actions/runs/12855797074/job/35841742982?pr=5212#step:4:20)89
/charts/prometheus-postgres-exporter/ @gianrubio @zanhsieh @zeritti
/charts/prometheus-pushgateway/ @cstaud @gianrubio @zeritti
/charts/prometheus-rabbitmq-exporter/ @desaintmartin @iamabhishek-dubey @juanchimienti @monotek
/charts/prometheus-redis-exporter/ @acondrat @zanhsieh
/charts/prometheus-smartctl-exporter/ @kfox1111 @zeritti
/charts/prometheus-snmp-exporter/ @miouge1 @walker-tom @xiu
/charts/prometheus-sql-exporter/ @wilfriedroset
/charts/prometheus-stackdriver-exporter/ @apenney @rpahli
/charts/prometheus-statsd-exporter/ @scDisorder
/charts/prometheus-systemd-exporter/ @capuche[24](https://github.com/prometheus-community/helm-charts/actions/runs/12855797074/job/35841742982?pr=5212#step:4:25)12 @maxime1907
/charts/prometheus-to-sd/ @acondrat
/charts/prometheus-windows-exporter/ @jkroepke
/charts/prometheus-yet-another-cloudwatch-exporter/ @cristiangreco @thomaspeitz
/charts/prometheus/ @gianrubio @naseemkullah @Xtigyro @zanhsieh @zeritti

but it seems that - (code 45) comes before / (code 47).

However it seems that the file inside the PR is not sorted which result in a diff.

@SuperQ SuperQ force-pushed the superq/update_script branch from bc1fe3c to 7c9ef3d Compare January 19, 2025 18:16
@SuperQ
Copy link
Contributor Author

SuperQ commented Jan 19, 2025

When I run the script locally, I get a different order result.

@SuperQ SuperQ force-pushed the superq/update_script branch 2 times, most recently from c6fd3b6 to 31b478e Compare January 19, 2025 20:43
@SuperQ
Copy link
Contributor Author

SuperQ commented Jan 19, 2025

There, I simplified the script down to one line of yq | sort 😹

@SuperQ SuperQ requested a review from jkroepke January 19, 2025 20:44
@SuperQ SuperQ force-pushed the superq/update_script branch from 31b478e to 9c432a6 Compare January 19, 2025 20:47
@SuperQ
[ci] Update codeowners script
8d8ab0d 
* Fixup issue link to point to correct location.
* Remove `@jkroepke` from manual listing now that they are a member of `helm-chart-admins`.
* Refactor `scripts/check-codeowners.sh` to simplify it.

Signed-off-by: SuperQ <superq@gmail.com>
@SuperQ SuperQ force-pushed the superq/update_script branch from 9c432a6 to 8d8ab0d Compare January 19, 2025 20:57
jkroepke
jkroepke previously approved these changes Jan 20, 2025
View reviewed changes
Copy link
Member

@jkroepke jkroepke left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally and works on my machine. Great, that you found a simpler way. Sounds like a good template for the MAINTAINER.md as well.

./scripts/check-codeowners.sh > .github/CODEOWNERS
# git diff is empty.

.editorconfig Show resolved Hide resolved
@SuperQ @jkroepke
Update .editorconfig
ff73c0b 
Mark as root to enforce repo settings.

Co-authored-by: Jan-Otto Kröpke <github@jkroepke.de>
Signed-off-by: Ben Kochie <superq@gmail.com>
@SuperQ SuperQ requested a review from jkroepke January 20, 2025 08:14
jkroepke
jkroepke approved these changes Jan 20, 2025
View reviewed changes
Copy link
Member

@jkroepke jkroepke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jkroepke jkroepke merged commit f726405 into main Jan 20, 2025
4 checks passed
@jkroepke jkroepke deleted the superq/update_script branch January 20, 2025 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkroepke jkroepke jkroepke approved these changes

@GMartinez-Sisti GMartinez-Sisti Awaiting requested review from GMartinez-Sisti GMartinez-Sisti is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SuperQ @jkroepke