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 for each CODEOWNERS file change #1315

Merged
merged 4 commits into from
Aug 21, 2024

Conversation

mszostok
Copy link
Contributor

@mszostok mszostok commented Jul 19, 2024

Description

  • This PR provides automation for updating the "MAINTAINERS.yaml" file in the community repository.

Notes for Reviewer

  1. Please watch the attached demo recordings:

  2. Please check the README.md which describes how it works.

    Click to see the flow diagram

    sequenceDiagram
        Refresher->>Refresher: Load API calls cache 
    
        note over Refresher, GitHub: Uses GraphQL
        Refresher->>+GitHub: Get all AsyncAPI repositories
        
        GitHub-->>-Refresher: Repository list
        
    
        Refresher->>Refresher: "Remove ignored repositories"
    
        rect rgb(191, 223, 255)
        note over Refresher, GitHub: Uses REST API
        
        loop For each repository
            Refresher->>+GitHub: Get CODEOWNERS file
            note over  Refresher: Detect the default branch
            note over  Refresher: Check for file in all valid locations
            GitHub-->>-Refresher: CODEOWNERS content
        end
        end
    
        rect rgb(191, 223, 255)
        note over Refresher, GitHub: Uses REST API
        
        Refresher->>Refresher: "Remove ignored users"
    
        loop For each unique codeowner
            Refresher->>+GitHub: Get codeowner GitHub profile
            GitHub-->>-Refresher: GitHub profile
        end
        end
    
    
        loop For previous maintainer
            alt is invalid
            Refresher->>Refresher: Delete if either GitHub profile was removed <br> or they doesn't own any repository currently.
            else is valid
            Refresher->>Refresher: Refresh only the 'repos' property.
            end
        end
    
        Refresher->>Refresher: Append any new maintainers not present in the previous list.
    
        Refresher->>Refresher: Prints details on why a maintainer was removed or changed directly
    
        Refresher->>Refresher: Save API calls cache
    
    Loading

  3. I used the rtCamp/action-slack-notify action; however, I also saw that you use 8398a7/action-slack. Let me know if I should switch to it.

  4. To keep the code simple, a full refresh is performed. This doesn't introduce any additional risks:

    • The logic takes up to 30 seconds only.
    • Hitting API rate limits is also not an issue:
      • The rate limit for GITHUB_TOKEN is 1,000 requests per hour per repository.
      • The update job executes 126 requests. However, thanks to conditional requests, for the next run, if there are no changes, all those calls don't count against your rate limit. So, if there is a CODEOWNERS file change from a single repository, it will use just 1 call.

Related Issue(s)

Blocked by: #1314 and asyncapi/github-action-for-cli#397

Fixes: #1269

@aeworxet
Copy link
Contributor

@asyncapi/bounty_team

@mszostok mszostok force-pushed the refresh-maintainers branch 3 times, most recently from 88b7c63 to 01322ca Compare July 23, 2024 21:42
@mszostok mszostok marked this pull request as ready for review July 23, 2024 21:43
@derberg
Copy link
Member

derberg commented Jul 27, 2024

don't worry about what actions for slack notifications is better -> we are not very consistent in this area 😄 -> https://github.com/asyncapi/.github/blob/master/.github/workflows/automerge-orphans.yml#L61

@mszostok mszostok force-pushed the refresh-maintainers branch from 01322ca to 255d2e7 Compare July 28, 2024 20:08
@mszostok
Copy link
Contributor Author

mszostok commented Jul 28, 2024

Thank you for the review! 🙇

Here is the 255d2e7 commit that contains the following changes:

  • rebase with master
  • use peter-evans/create-pull-request instead of git + gh pr
  • use github-script and remove package.json, and other config related files
  • add short "about" section directly in update-maintainers.yaml job definition
  • introduce githubID prop, so in the next PR we can switch from comparing GitHub usernames to more reliable solution comparing GitHub profile id as specified here.

Here is the GitHub run where I tested the all provided changes: https://github.com/refresh-maintainers-demo/community/actions/runs/10134304231/job/28020379271

@mszostok mszostok requested a review from derberg July 30, 2024 06:14
@mszostok
Copy link
Contributor Author

mszostok commented Aug 2, 2024

Hi 👋
Here is the fde9775 commit that contains the following changes:

  • take into account the usernames under #docTriagers and #codeTriagers as specified in e.g. website repo
    • as you can see in this diff the TRohit20 BhaswatiRoy VaishnaviNandakumar J0SAL were added while sambhavgupta0705 was not removed.
  • update the cron to run at 10:00 AM UTC every Sunday.

Here is the GitHub run where I tested the all provided changes: https://github.com/refresh-maintainers-demo/community/actions/runs/10211359115/job/28252539643

Cheers!

@aeworxet
Copy link
Contributor

/ptal

@asyncapi-bot
Copy link
Contributor

@thulieblack Please take a look at this PR. Thanks! 👋

@aeworxet
Copy link
Contributor

I was PTALing mostly @derberg, but the code is good; why not make everyone look at it, sure?

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

Thanks @mszostok !!! and sorry for delay, holidays 😉

@derberg
Copy link
Member

derberg commented Aug 21, 2024

/rtm

@asyncapi-bot asyncapi-bot merged commit 1c2f236 into asyncapi:master Aug 21, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

complete automation related to having MAINTAINERS.yaml always up to date with current state
4 participants