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

[all] added MAINTAINERS.md #5083

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

dotdc
Copy link
Member

@dotdc dotdc commented Dec 21, 2024

What this PR does / why we need it

Here's a proposal for MAINTAINERS.md as asked in #5026.
Just kept GitHub handles or forename when I didn't manage to find the full name (maybe it's on purpose):

We can adapt if needed.

cc @RichiH

Which issue this PR 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])

Signed-off-by: David Calvert <[email protected]>
@dotdc dotdc requested a review from a team as a code owner December 21, 2024 19:51
MAINTAINERS.md Outdated Show resolved Hide resolved
@jkroepke
Copy link
Member

A automation would this would be pretty. Guessing the real name could be fetched via GH API.

Co-authored-by: Jan-Otto Kröpke <[email protected]>
Signed-off-by: David Calvert <[email protected]>
@dotdc
Copy link
Member Author

dotdc commented Dec 21, 2024

A automation would this would be pretty. Guessing the real name could be fetched via GH API.

True, but we don't add maintainers very often, and not all names are present in GitHub.
We can add a comment in CODEOWNERS eventually.

@jkroepke
Copy link
Member

Could you bump the python version in the CI in a separate PR? At least, I head merge permissions to fix that.

@dotdc
Copy link
Member Author

dotdc commented Dec 22, 2024

Could you bump the python version in the CI in a separate PR? At least, I head merge permissions to fix that.

Created #5085 with Python 3.11

Merged ✔️

@DrFaust92
Copy link
Contributor

On A semi related note, how does one ask for being a maintainer? I would like to offer a hand if I could based on previous contribs

@dotdc
Copy link
Member Author

dotdc commented Dec 22, 2024

On A semi related note, how does one ask for being a maintainer? I would like to offer a hand if I could based on previous contribs

@DrFaust92 Awesome! You should open an issue and PR(s) like #3431.

Copy link
Contributor

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

I would like a script to keep this file and CODEOWNERS in sync. One of them should be a source of truth.

@dotdc
Copy link
Member Author

dotdc commented Jan 13, 2025

I would like a script to keep this file and CODEOWNERS in sync. One of them should be a source of truth.

Not sure if it's worth it since we don't add maintainers very often, and several chart maintainers don't include their names in their GitHub profiles.

We could consider the following options:

  • Generate the file using only GitHub handles, output will be short and redundant with CODEOWNERS.
  • Generate the file with some inconsistencies: include names and GitHub handles when available, and GitHub handles only when the names are missing.
  • Manually hardcode missing names in the script.

Nothing ideal, but we could pick one to have something at least.
Any thoughts or ideas?

cc @SuperQ

@dotdc dotdc requested a review from SuperQ January 13, 2025 12:19
@jkroepke
Copy link
Member

jkroepke commented Jan 13, 2025

One of them should be a source of truth.

tbh. since CODEOWNERS are just generated from Chart.yaml, the Chart.yaml are the source of truth here. + admins.

We can start simple. Omit the full name and just add the GitHub handle should be sufficient.

@SuperQ
Copy link
Contributor

SuperQ commented Jan 13, 2025

This should do most of the work:

#!/usr/bin/env bash

cat <<EOF
# Maintainers

## General maintainers

- André Bauer (@monotek)
- Scott Rigby (@scottrigby)
- Torsten Walter (@torstenwalter)

## GitHub Workflows & Renovate maintainers

- Jan-Otto Kröpke (@jkroepke)
- Gabriel Martinez (@GMartinez-Sisti)

## Helm charts maintainers

EOF

for chart in $(ls -1 -d ./charts/*)
do
  chart_file="${chart}/Chart.yaml"
  chart=$(echo ${chart} | sed 's/^\.//')
  echo "### ${chart}"
  echo ''
  yq e '.maintainers[] | ("- <" + .email + "> @" + .name)' "${chart_file}" | sort --ignore-case
  echo ''
done

We could change the way we use name to be "full name" and make use of the maintainers url spec be https://github.com/`.

@jkroepke
Copy link
Member

jkroepke commented Jan 13, 2025

I like the script.

About the full name: in times of GDPR and data economy, the full name of each maintainer shouldn't be mandatory. From maintainer info perspective, the GitHub handle + mail is fully sufficient.

@SuperQ
Copy link
Contributor

SuperQ commented Jan 13, 2025

We don't require it as a policy, but we make it an option for contact formatting of our MAINTAINERS files. People can choose whatever name field they want.

* Name <email> @handle

@jkroepke
Copy link
Member

Ok, to summarize the proposal from SuperQ:

  1. Change the maintainer info in all helm charts to the following schema:
# Chart.yaml
apiVersion: 2
# ...
maintainers: 
  - name: The maintainers name # (required for each maintainer) - free text
    email: The maintainers email # will be not changed
    url: A URL for the maintainer # link to GitHub profile. 
  1. Change the CODEOWNER script. (scrape the github handle from the URL field) and generate a MAINTAINER file as well.

While it looks a bit over-engineered first, it giving individual maintainers the possibility the add they correct name. Thats a benefit for this eco system.

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.

[Janitorial] Please add MAINTAINERS.md
5 participants