-
-
Notifications
You must be signed in to change notification settings - Fork 107
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: verify changes to Maintainers.yaml made by the bot #750
ci: verify changes to Maintainers.yaml made by the bot #750
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you prefer bash over js 😄
- why do we need two
POST
calls? call toreviews
endpoint should be enough, no? - it is not such simple validation of the actor. We discussed that there is a case for human edit, when people want to update some information about them, like linkedin, or company affiliation for example. So validation should block PR from merging only if human creates a PR that adds or remove new maintainer object imho
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see PR title verify changes to Maintainers.yaml made by the bot
but no code that verifies that changes are made by bot. Can't review as not sure what is the context an cannot understand why bot cannot remove maintainer and that PR is closed
Thank you @derberg for pointing this out. It was an oversight, and I've now added the check to verify if changes are made by the bot. |
Co-authored-by: Lukasz Gornicki <lpgornicki@gmail.com>
Co-authored-by: Lukasz Gornicki <lpgornicki@gmail.com>
Co-authored-by: Lukasz Gornicki <lpgornicki@gmail.com>
Co-authored-by: Lukasz Gornicki <lpgornicki@gmail.com>
Thanks for the review. I have updated the description with recent logs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one small thing, other than that, looks great!
Co-authored-by: Khuda Dad Nomani <32505158+KhudaDad414@users.noreply.github.com>
Thanks @KhudaDad414, I have made the requested changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
shouldn't this fail 14Richa/testRepo#417 ? |
…munity into 14Richa/validate-maintainers
Yep @derberg. I have made minor fixes. Now it's working fine. You can take a look here : 14Richa/testRepo#420 |
Co-authored-by: Lukasz Gornicki <lpgornicki@gmail.com>
…munity into 14Richa/validate-maintainers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏🏼
/dnm making sure it won't get merged by any automation to early |
…munity into 14Richa/validate-maintainers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
/rtm |
Description
This pull request includes a workflow triggered when changes are made to the
Maintainers.yaml
file. It checks for new maintainer objects, removes maintainer objects, and modifies GitHub usernames and repository lists.This workflow has 3 jobs:
verify-changes
: This job compares changes in theMAINTAINERS.yaml
file between the main branch and the pull request, identifies removed TSC members by theasyncapi-bot
, and checks for critical changes made by humans.handle-human-made-critical-changes
: If there are critical changes detected in theMAINTAINERS.yaml
file, this job posts a comment on the pull request with the error messages and closes the pull request.bot-removal-notification
: This job notifies maintainers when theasyncapi-bot
removes a TSCmember from the MAINTAINERS.yaml file by posting a comment on the pull request.I have tested the workflow in my testRepo you can find the logs : https://github.com/14Richa/testRepo/actions/runs/5750090267
Related issue(s)
#210