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

ci: update MAINTAINERS.yaml based on CODEOWNERS changes #248

Open
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

14Richa
Copy link
Contributor

@14Richa 14Richa commented Jul 12, 2023

Description

This PR updates the MAINTAINERS.yaml file based on the changes made in the CODEOWNERS file. The workflow takes the repository name as input and adds it to the MAINTAINERS.yaml file.

When a new code owner is added, a corresponding maintainer object is created in the MAINTAINERS.yaml file with the repository name and the Twitter username of the code owner added to the object. It fetches the Twitter username using the GitHub API.

Similarly, when a code owner is removed, the corresponding maintainer object is removed from the MAINTAINERS.yaml file. The removal process checks if the maintainer has any other repositories before removing them from the MAINTAINERS.yaml file.

I have tested this workflow on my testRepo by adding a new code owner, and the MAINTAINERS.yaml file was updated with the new code owner object and repository name. Here are the logs from the workflow: https://github.com/14Richa/testRepo/actions/runs/5563061192/jobs/10161768757

Similarly, when a code owner is removed MAINTAINERS.yaml file was updated accordingly with the maintainer object removed from the file. Here are the logs from the workflow: https://github.com/14Richa/testRepo/actions/runs/5529277200/jobs/10087118152

@14Richa 14Richa marked this pull request as ready for review July 12, 2023 15:04
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

yo, initial review for now, main algorithm review once we get basics fixed

.github/workflows/update-maintainers.yml Outdated Show resolved Hide resolved
.github/workflows/update-maintainers.yml Outdated Show resolved Hide resolved
.github/workflows/update-maintainers.yml Outdated Show resolved Hide resolved
.github/workflows/update-maintainers.yml Outdated Show resolved Hide resolved
Copy link
Member

@KhudaDad414 KhudaDad414 left a comment

Choose a reason for hiding this comment

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

@14Richa
Copy link
Contributor Author

14Richa commented Jul 17, 2023

getting syntax error https://github.com/KhudaDad414/.github/actions/runs/5574259435/jobs/10182640765

Fixed it. Please check now!

Copy link
Member

@KhudaDad414 KhudaDad414 left a comment

Choose a reason for hiding this comment

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

Isn't this PR supposed to open a pull request to the community repo? I see it applies the changes locally.

.github/workflows/update-maintainers.yml Outdated Show resolved Hide resolved
.github/workflows/update-maintainers.yml Outdated Show resolved Hide resolved
.github/workflows/update-maintainers.yml Outdated Show resolved Hide resolved
.github/workflows/update-maintainers.yml Outdated Show resolved Hide resolved
@14Richa
Copy link
Contributor Author

14Richa commented Jul 18, 2023

Isn't this PR supposed to open a pull request to the community repo? I see it applies the changes locally.

Thank you for bringing this to my attention. I have updated the workflow. Now it creates a new branch and pushes the changes there. Subsequently, it opens a pull request for these changes to be reviewed and merged into the master branch in the community repo.

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

some cleanup needed

.github/workflows/update-maintainers.yml Outdated Show resolved Hide resolved
.github/workflows/update-maintainers.yml Show resolved Hide resolved
.github/workflows/update-maintainers.yml Outdated Show resolved Hide resolved
.github/workflows/update-maintainers.yml Show resolved Hide resolved
.github/workflows/update-maintainers.yml Outdated Show resolved Hide resolved
.github/workflows/update-maintainers.yml Show resolved Hide resolved
Copy link
Member

@KhudaDad414 KhudaDad414 left a comment

Choose a reason for hiding this comment

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

Looks great. 👍

.github/workflows/update-maintainers.yml Outdated Show resolved Hide resolved
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

2 comments only

.github/workflows/update-maintainers.yml Outdated Show resolved Hide resolved
.github/workflows/update-maintainers.yml Outdated Show resolved Hide resolved
derberg
derberg previously approved these changes Jul 25, 2023
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

LGTM

before we merge, we need to manually add it to generator repo for testing

Co-authored-by: Lukasz Gornicki <lpgornicki@gmail.com>
@14Richa
Copy link
Contributor Author

14Richa commented Jul 25, 2023

before we merge, we need to manually add it to generator repo for testing

@derberg , should I raise PR in the generator repo?

KhudaDad414
KhudaDad414 previously approved these changes Jul 26, 2023
Copy link
Member

@KhudaDad414 KhudaDad414 left a comment

Choose a reason for hiding this comment

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

LGTM!

derberg
derberg previously approved these changes Jul 27, 2023
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

lgtm

@derberg
Copy link
Member

derberg commented Jul 27, 2023

/dnm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants