[llvm] [workflows] Add a new workflow for checking commit access qualifications (PR #93301)

Tom Stellard via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 18 13:39:34 PDT 2024


https://github.com/tstellar updated https://github.com/llvm/llvm-project/pull/93301

>From defbfa83dc5e7f4303a68b4387b0c760dc39395b Mon Sep 17 00:00:00 2001
From: Tom Stellard <tstellar at redhat.com>
Date: Thu, 23 May 2024 16:39:28 -0700
Subject: [PATCH 1/7] [workflows] Add a new workflow for checking commit access
 qualifications

This workflow produces a list of users that should be moved to the LLVM
Triagers team.  An admin will review the list and then manually move
people to the LLVM Triagers Team.

I've also added some documentation to the Developer Policy about the
Triage Role.
---
 .github/workflows/commit-access-review.py  | 427 +++++++++++++++++++++
 .github/workflows/commit-access-review.yml |  34 ++
 llvm/docs/DeveloperPolicy.rst              |  14 +
 3 files changed, 475 insertions(+)
 create mode 100644 .github/workflows/commit-access-review.py
 create mode 100644 .github/workflows/commit-access-review.yml

diff --git a/.github/workflows/commit-access-review.py b/.github/workflows/commit-access-review.py
new file mode 100644
index 0000000000000..34f0f637fd2b6
--- /dev/null
+++ b/.github/workflows/commit-access-review.py
@@ -0,0 +1,427 @@
+import datetime
+import github
+import re
+import requests
+import time
+import sys
+import re
+
+
+class User:
+
+    THRESHOLD = 5
+
+    def __init__(self, name, triage_list):
+        self.name = name
+        self.authored = 0
+        self.merged = 0
+        self.reviewed = 0
+        self.triage_list = triage_list
+    
+    def add_authored(self, val = 1):
+        self.authored += val
+        if self.meets_threshold():
+            print(self.name, "meets the threshold with authored commits")
+            del self.triage_list[self.name]
+
+    def set_authored(self, val):
+        self.authored = 0
+        self.add_authored(val)
+
+    def add_merged(self, val = 1):
+        self.merged += val
+        if self.meets_threshold():
+            print(self.name, "meets the threshold with merged commits")
+            del self.triage_list[self.name]
+
+    def add_reviewed(self, val = 1):
+        self.reviewed += val
+        if self.meets_threshold():
+            print(self.name, "meets the threshold with reviewed commits")
+            del self.triage_list[self.name]
+
+    def get_total(self):
+        return self.authored + self.merged + self.reviewed
+
+    def meets_threshold(self):
+        return self.get_total() >= self.THRESHOLD
+
+    def __repr__(self):
+        return "{} : a: {} m: {} r: {}".format(self.name, self.authored, self.merged, self.reviewed)
+
+def run_graphql_query(query: str, variables: dict, token: str, retry: bool = True) -> dict:
+    """
+    This function submits a graphql query and returns the results as a
+    dictionary.
+    """
+    s = requests.Session()
+    retries = requests.adapters.Retry(total = 8, backoff_factor = 2, status_forcelist = [504])
+    s.mount('https://', requests.adapters.HTTPAdapter(max_retries = retries))
+
+    headers = {
+        'Authorization' : 'bearer {}'.format(token),
+        # See
+        # https://github.blog/2021-11-16-graphql-global-id-migration-update/
+        'X-Github-Next-Global-ID': '1'
+    }
+    request = s.post(
+        url = 'https://api.github.com/graphql',
+        json = {"query" : query, "variables" : variables },
+        headers = headers)
+
+    rate_limit = request.headers.get('X-RateLimit-Remaining')
+    print(rate_limit)
+    if rate_limit and int(rate_limit) < 10:
+        reset_time = int(request.headers['X-RateLimit-Reset'])
+        while reset_time - int(time.time()) > 0:
+            time.sleep(60)
+            print("Waiting until rate limit reset", reset_time - int(time.time()), 'seconds remaining')
+
+    if request.status_code == 200:
+        if 'data' not in request.json():
+            print(request.json())
+            sys.exit(1)
+        return request.json()['data']
+    elif retry:
+        return run_graphql_query(query, variables, token, False)
+    else:
+        raise Exception(
+            "Failed to run graphql query\nquery: {}\nerror: {}".format(query, request.json()))
+
+
+def check_manual_requests(start_date, token) -> bool:
+    query = """
+        query ($query: String!) {
+          search(query: $query, type: ISSUE, first: 100) {
+            nodes {
+              ... on Issue {
+                body
+                comments (first: 100) {
+                  nodes {
+                    author {
+                      login
+                    }
+                  }
+                }
+              }
+            }
+          }
+        }
+        """
+    formatted_start_date = start_date.strftime("%Y-%m-%dT%H:%M:%S")
+    variables = {
+        "query" : f"type:issue created:>{formatted_start_date} org:llvm repo:llvm-project label:infrastructure:commit-access"
+    }
+
+    data = run_graphql_query(query, variables, token)
+    users = []
+    for issue in data['search']['nodes']:
+        users.extend([user[1:] for user in re.findall('@[^ ,\n]+', issue['body'])])
+        # Do we need to check comments if we are checking mentions??
+        #for comment in issue['comments']['nodes']:
+        #    users.append(comment['author']['login'])
+
+    return users
+
+
+def get_num_commits(user, start_date, token) -> bool:
+    variables = {
+        "owner" : 'llvm',
+        'user' : user,
+        'start_date' : start_date.strftime("%Y-%m-%dT%H:%M:%S")
+    }
+
+    user_query = """
+        query ($user: String!) {
+          user(login: $user) {
+            id
+          }
+        }
+    """
+
+    data = run_graphql_query(user_query, variables, token)
+    variables['user_id'] = data['user']['id']
+
+    query = """
+        query ($owner: String!, $user_id: ID!, $start_date: GitTimestamp!){
+          organization(login: $owner) {
+            teams(query: "llvm-committers" first:1) {
+              nodes {
+                repositories {
+                  nodes {
+                    ref(qualifiedName: "main") {
+                      target {
+                        ... on Commit {
+                          history(since: $start_date, author: {id: $user_id }) {
+                            totalCount
+                          }
+                        }
+                      }
+                    }
+                  }
+                }
+              }
+            }
+          }
+        }
+     """
+    count = 0
+    data = run_graphql_query(query, variables, token)
+    for repo in data['organization']['teams']['nodes'][0]['repositories']['nodes']:
+        count += int(repo['ref']['target']['history']['totalCount'])
+        if count >= User.THRESHOLD:
+            break
+    return count
+
+def is_new_committer_query_user(user, start_date, token):
+    user_query = """
+        query {
+          organization(login: "llvm") {
+            id
+          }
+        }
+    """
+
+    data = run_graphql_query(user_query, {}, token)
+    variables = {
+        # We can only check one year of date at a time, so check for contribution between 3 and 4 years ago.
+        "start_date" : (start_date - datetime.timedelta(weeks = 2 * 52)).strftime("%Y-%m-%dT%H:%M:%S"),
+        "org" : data['organization']['id'],
+        "user" : user
+    }
+    query = """
+        query ($user: String!, $start_date: DateTime!, $org:ID!){
+          user(login: $user) {
+            contributionsCollection(from:$start_date, organizationID:$org) {
+                totalCommitContributions
+            }
+          }
+        }
+    """
+
+    data = run_graphql_query(query, variables, token)
+    if int(data['user']['contributionsCollection']['totalCommitContributions']) > 0:
+        return False
+    return True
+
+def is_new_committer_query_repo(user, start_date, token):
+    variables = {
+        'user' : user,
+    }
+
+    user_query = """
+        query ($user: String!) {
+          user(login: $user) {
+            id
+          }
+        }
+    """
+
+    data = run_graphql_query(user_query, variables, token)
+    variables['owner'] = 'llvm'
+    variables['user_id'] = data['user']['id']
+    variables['start_date'] = start_date.strftime("%Y-%m-%dT%H:%M:%S")
+
+    query = """
+        query ($owner: String!, $user_id: ID!){
+          organization(login: $owner) {
+            repository(name: "llvm-project") {
+              ref(qualifiedName: "main") {
+                target {
+                  ... on Commit {
+                    history(author: {id: $user_id }, first: 5) {
+                      nodes {
+                        committedDate
+                      }
+                    }
+                  }
+                }
+              }
+            }
+          }
+        }
+     """
+
+    data = run_graphql_query(query, variables, token)
+    repo = data['organization']['repository']
+    commits = repo['ref']['target']['history']['nodes']
+    if len(commits) == 0:
+        return True
+    committed_date = commits[-1]['committedDate']
+    if datetime.datetime.strptime(committed_date, "%Y-%m-%dT%H:%M:%SZ") < start_date:
+        return False
+    return True
+
+def is_new_committer_pr_author(user, start_date, token):
+    query = """
+        query ($query: String!) {
+          search(query: $query, type: ISSUE, first: 5) {
+            issueCount
+          }
+        }
+        """
+    formatted_start_date = start_date.strftime("%Y-%m-%dT%H:%M:%S")
+    variables = {
+        "owner" : "llvm",
+        "repo" : "llvm-project",
+        "user" : user,
+        "query" : f"type:pr author:{user} created:>{formatted_start_date} org:llvm"
+    }
+
+    data = run_graphql_query(query, variables, token)
+    return int(data['search']['issueCount']) > 0
+
+
+def is_new_committer(user, start_date, token):
+    try:
+        return is_new_committer_query_repo(user, start_date, token)
+    except:
+        pass
+    return True
+
+def get_review_count(user, start_date, token):
+
+    query = """
+        query ($query: String!) {
+          search(query: $query, type: ISSUE, first: 5) {
+            issueCount
+          }
+        }
+        """
+    formatted_start_date = start_date.strftime("%Y-%m-%dT%H:%M:%S")
+    variables = {
+        "owner" : "llvm",
+        "repo" : "llvm-project",
+        "user" : user,
+        "query" : f"type:pr commenter:{user} -author:{user} merged:>{formatted_start_date} org:llvm"
+    }
+
+    data = run_graphql_query(query, variables, token)
+    return int(data['search']['issueCount'])
+
+
+def count_prs(triage_list, start_date, token):
+    query = """
+        query ($query: String!, $after: String) {
+          search(query: $query, type: ISSUE, first: 100, after: $after) {
+            issueCount,
+            nodes {
+              ... on PullRequest {
+                 author {
+                   login
+                 }
+                 mergedBy {
+                   login
+                 }
+              }
+            }
+            pageInfo {
+              hasNextPage
+              endCursor
+            }
+          }
+        }
+    """
+    date_begin = start_date
+    date_end = None
+    while date_begin < datetime.datetime.now():
+        date_end = date_begin + datetime.timedelta(days = 7)
+        formatted_date_begin = date_begin.strftime("%Y-%m-%dT%H:%M:%S")
+        formatted_date_end = date_end.strftime("%Y-%m-%dT%H:%M:%S")
+        variables = {
+          "query" : f"type:pr is:merged merged:{formatted_date_begin}..{formatted_date_end} org:llvm",
+        }
+        has_next_page = True
+        while has_next_page:
+            print(variables)
+            data = run_graphql_query(query, variables, token)
+            for pr in data['search']['nodes']:
+                # Users can be None if the user has been deleted.
+                if not pr['author']:
+                    continue
+                author = pr['author']['login']
+                if author in triage_list:
+                    triage_list[author].add_authored()
+
+                if not pr['mergedBy']:
+                    continue
+                merger = pr['mergedBy']['login']
+                if author == merger:
+                    continue
+                if merger not in triage_list:
+                    continue
+                triage_list[merger].add_merged()
+
+            has_next_page = data['search']['pageInfo']['hasNextPage']
+            if has_next_page:
+                variables['after'] = data['search']['pageInfo']['endCursor']
+        date_begin = date_end
+
+
+def main():
+
+    token = sys.argv[1]
+    gh = github.Github(login_or_token=token)
+    org = gh.get_organization('llvm')
+    repo = org.get_repo('llvm-project')
+    team = org.get_team_by_slug('llvm-committers')
+    one_year_ago = datetime.datetime.now() - datetime.timedelta(days = 365)
+    triage_list = {}
+    for member in team.get_members():
+        triage_list[member.login] = User(member.login, triage_list)
+
+    print("Start:", len(triage_list), "triagers")
+    # Step 0 Check if users have requested commit access in the last year.
+
+    for user in check_manual_requests(one_year_ago, token):
+        if user in triage_list:
+            print(user, "requested commit access in the last year.")
+            del triage_list[user]
+    print("After Request Check:", len(triage_list), "triagers")
+
+    # Step 1 count all PRs authored or merged
+    count_prs(triage_list, one_year_ago, token)
+
+    print("After PRs:", len(triage_list), "triagers")
+
+    if len(triage_list) == 0:
+        sys.exit(0)
+
+    # Step 2 check for reviews
+    for user in list(triage_list.keys()):
+        review_count = get_review_count(user, one_year_ago, token)
+        triage_list[user].add_reviewed(review_count)
+
+    print("After Reviews:", len(triage_list), "triagers")
+
+    if len(triage_list) == 0:
+        sys.exit(0)
+
+    # Step 3 check for number of commits
+    for user in list(triage_list.keys()):
+        num_commits = get_num_commits(user, one_year_ago, token)
+        # Override the total number of commits to not double count commits and
+        # authored PRs.
+        triage_list[user].set_authored(num_commits)
+
+    print("After Commits:", len(triage_list), "triagers")
+
+    # Step 4 check for new committers
+    for user in list(triage_list.keys()):
+        print ("Checking", user)
+        if is_new_committer(user, one_year_ago, token):
+            print("Removing new committer: ", user)
+            del triage_list[user]
+
+    print("Complete:", len(triage_list), "triagers")
+
+    filename = 'triagers.log'
+    f = open(filename, 'w')
+    for user in triage_list:
+        print(triage_list[user].__repr__())
+        f.write(user + "\n")
+    f.close()
+
+
+if __name__ == "__main__":
+    main()
diff --git a/.github/workflows/commit-access-review.yml b/.github/workflows/commit-access-review.yml
new file mode 100644
index 0000000000000..f9195a1863dee
--- /dev/null
+++ b/.github/workflows/commit-access-review.yml
@@ -0,0 +1,34 @@
+name: Commit Access Review
+
+on:
+  workflow_dispatch:
+  schedule:
+    # * is a special character in YAML so you have to quote this string
+    - cron:  '0 7 1 * *'
+
+permissions:
+  contents: read
+
+jobs:
+  commit-access-review:
+    if: github.repository_owner == 'llvm'
+    runs-on: ubuntu-22.04
+    steps:
+      - name: Fetch LLVM sources
+        uses: actions/checkout at v4
+      
+      - name: Install dependencies
+        run: |
+          pip install --require-hashes -r ./llvm/utils/git/requirements.txt
+      
+      - name: Run Script
+        env:
+          GITHUB_TOKEN: ${{ secrets.RELEASE_TASKS_USER_TOKEN }}
+        run: |
+          python3 .github/workflows/commit-access-review.py $GITHUB_TOKEN
+
+      - name: Upload Triage List
+        uses: actions/upload-artifact at 26f96dfa697d77e81fd5907df203aa23a56210a8 #v4.3.0
+        with:
+          name: triagers
+          path: triagers.log
diff --git a/llvm/docs/DeveloperPolicy.rst b/llvm/docs/DeveloperPolicy.rst
index f2ac46e6c04f2..1f3b91421fb2f 100644
--- a/llvm/docs/DeveloperPolicy.rst
+++ b/llvm/docs/DeveloperPolicy.rst
@@ -527,6 +527,20 @@ after they are committed, depending on the nature of the change).  You are
 encouraged to review other peoples' patches as well, but you aren't required
 to do so.
 
+Triage Role
+^^^^^^^^^^^^^
+
+The `Triage Role <https://docs.github.com/en/organizations/managing-user-access-to-your-organizations-repositories/managing-repository-roles/repository-roles-for-an-organization>`_ in GitHub gives users elevated permissions for managing issues and
+pull requests.  If you want to help triaging issues and pull requests in the
+project and don't need write access to the repository, you may request triage
+access instead of commit access when you email Chris.
+
+Also, once a month we review the list of users with commit access and ask anyone
+(via a GitHub issue) with less than 5 interactions (i.e commits, created PRs, or
+PR comments) if they  still need commit access.  If a user does not respond within
+30 days, then they will be moved from the Write Role to the Triage Role.
+
+
 .. _discuss the change/gather consensus:
 
 Making a Major Change

>From 726322b65fcf76df4b06ba734bb33f343fe2ede9 Mon Sep 17 00:00:00 2001
From: Tom Stellard <tstellar at redhat.com>
Date: Fri, 24 May 2024 06:51:52 -0700
Subject: [PATCH 2/7] Fix typos in the documentation

---
 llvm/docs/DeveloperPolicy.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/llvm/docs/DeveloperPolicy.rst b/llvm/docs/DeveloperPolicy.rst
index 1f3b91421fb2f..43e1141edb82d 100644
--- a/llvm/docs/DeveloperPolicy.rst
+++ b/llvm/docs/DeveloperPolicy.rst
@@ -531,9 +531,9 @@ Triage Role
 ^^^^^^^^^^^^^
 
 The `Triage Role <https://docs.github.com/en/organizations/managing-user-access-to-your-organizations-repositories/managing-repository-roles/repository-roles-for-an-organization>`_ in GitHub gives users elevated permissions for managing issues and
-pull requests.  If you want to help triaging issues and pull requests in the
-project and don't need write access to the repository, you may request triage
-access instead of commit access when you email Chris.
+pull requests.  If you want to help triage issues and pull requests in the
+project and don't need commit access, you may request triage access instead
+of commit access when you email Chris.
 
 Also, once a month we review the list of users with commit access and ask anyone
 (via a GitHub issue) with less than 5 interactions (i.e commits, created PRs, or

>From 54639ee0bc4349ce844bc7d65230b14681ef4a7e Mon Sep 17 00:00:00 2001
From: Tom Stellard <tstellar at redhat.com>
Date: Fri, 24 May 2024 13:29:59 -0700
Subject: [PATCH 3/7] Fix python formatting

---
 .github/workflows/commit-access-review.py | 154 ++++++++++++----------
 1 file changed, 85 insertions(+), 69 deletions(-)

diff --git a/.github/workflows/commit-access-review.py b/.github/workflows/commit-access-review.py
index 34f0f637fd2b6..3a01ab570f4a1 100644
--- a/.github/workflows/commit-access-review.py
+++ b/.github/workflows/commit-access-review.py
@@ -8,7 +8,6 @@
 
 
 class User:
-
     THRESHOLD = 5
 
     def __init__(self, name, triage_list):
@@ -17,8 +16,8 @@ def __init__(self, name, triage_list):
         self.merged = 0
         self.reviewed = 0
         self.triage_list = triage_list
-    
-    def add_authored(self, val = 1):
+
+    def add_authored(self, val=1):
         self.authored += val
         if self.meets_threshold():
             print(self.name, "meets the threshold with authored commits")
@@ -28,13 +27,13 @@ def set_authored(self, val):
         self.authored = 0
         self.add_authored(val)
 
-    def add_merged(self, val = 1):
+    def add_merged(self, val=1):
         self.merged += val
         if self.meets_threshold():
             print(self.name, "meets the threshold with merged commits")
             del self.triage_list[self.name]
 
-    def add_reviewed(self, val = 1):
+    def add_reviewed(self, val=1):
         self.reviewed += val
         if self.meets_threshold():
             print(self.name, "meets the threshold with reviewed commits")
@@ -47,46 +46,59 @@ def meets_threshold(self):
         return self.get_total() >= self.THRESHOLD
 
     def __repr__(self):
-        return "{} : a: {} m: {} r: {}".format(self.name, self.authored, self.merged, self.reviewed)
+        return "{} : a: {} m: {} r: {}".format(
+            self.name, self.authored, self.merged, self.reviewed
+        )
+
 
-def run_graphql_query(query: str, variables: dict, token: str, retry: bool = True) -> dict:
+def run_graphql_query(
+    query: str, variables: dict, token: str, retry: bool = True
+) -> dict:
     """
     This function submits a graphql query and returns the results as a
     dictionary.
     """
     s = requests.Session()
-    retries = requests.adapters.Retry(total = 8, backoff_factor = 2, status_forcelist = [504])
-    s.mount('https://', requests.adapters.HTTPAdapter(max_retries = retries))
+    retries = requests.adapters.Retry(total=8, backoff_factor=2, status_forcelist=[504])
+    s.mount("https://", requests.adapters.HTTPAdapter(max_retries=retries))
 
     headers = {
-        'Authorization' : 'bearer {}'.format(token),
+        "Authorization": "bearer {}".format(token),
         # See
         # https://github.blog/2021-11-16-graphql-global-id-migration-update/
-        'X-Github-Next-Global-ID': '1'
+        "X-Github-Next-Global-ID": "1",
     }
     request = s.post(
-        url = 'https://api.github.com/graphql',
-        json = {"query" : query, "variables" : variables },
-        headers = headers)
+        url="https://api.github.com/graphql",
+        json={"query": query, "variables": variables},
+        headers=headers,
+    )
 
-    rate_limit = request.headers.get('X-RateLimit-Remaining')
+    rate_limit = request.headers.get("X-RateLimit-Remaining")
     print(rate_limit)
     if rate_limit and int(rate_limit) < 10:
-        reset_time = int(request.headers['X-RateLimit-Reset'])
+        reset_time = int(request.headers["X-RateLimit-Reset"])
         while reset_time - int(time.time()) > 0:
             time.sleep(60)
-            print("Waiting until rate limit reset", reset_time - int(time.time()), 'seconds remaining')
+            print(
+                "Waiting until rate limit reset",
+                reset_time - int(time.time()),
+                "seconds remaining",
+            )
 
     if request.status_code == 200:
-        if 'data' not in request.json():
+        if "data" not in request.json():
             print(request.json())
             sys.exit(1)
-        return request.json()['data']
+        return request.json()["data"]
     elif retry:
         return run_graphql_query(query, variables, token, False)
     else:
         raise Exception(
-            "Failed to run graphql query\nquery: {}\nerror: {}".format(query, request.json()))
+            "Failed to run graphql query\nquery: {}\nerror: {}".format(
+                query, request.json()
+            )
+        )
 
 
 def check_manual_requests(start_date, token) -> bool:
@@ -110,15 +122,15 @@ def check_manual_requests(start_date, token) -> bool:
         """
     formatted_start_date = start_date.strftime("%Y-%m-%dT%H:%M:%S")
     variables = {
-        "query" : f"type:issue created:>{formatted_start_date} org:llvm repo:llvm-project label:infrastructure:commit-access"
+        "query": f"type:issue created:>{formatted_start_date} org:llvm repo:llvm-project label:infrastructure:commit-access"
     }
 
     data = run_graphql_query(query, variables, token)
     users = []
-    for issue in data['search']['nodes']:
-        users.extend([user[1:] for user in re.findall('@[^ ,\n]+', issue['body'])])
+    for issue in data["search"]["nodes"]:
+        users.extend([user[1:] for user in re.findall("@[^ ,\n]+", issue["body"])])
         # Do we need to check comments if we are checking mentions??
-        #for comment in issue['comments']['nodes']:
+        # for comment in issue['comments']['nodes']:
         #    users.append(comment['author']['login'])
 
     return users
@@ -126,9 +138,9 @@ def check_manual_requests(start_date, token) -> bool:
 
 def get_num_commits(user, start_date, token) -> bool:
     variables = {
-        "owner" : 'llvm',
-        'user' : user,
-        'start_date' : start_date.strftime("%Y-%m-%dT%H:%M:%S")
+        "owner": "llvm",
+        "user": user,
+        "start_date": start_date.strftime("%Y-%m-%dT%H:%M:%S"),
     }
 
     user_query = """
@@ -140,7 +152,7 @@ def get_num_commits(user, start_date, token) -> bool:
     """
 
     data = run_graphql_query(user_query, variables, token)
-    variables['user_id'] = data['user']['id']
+    variables["user_id"] = data["user"]["id"]
 
     query = """
         query ($owner: String!, $user_id: ID!, $start_date: GitTimestamp!){
@@ -167,12 +179,13 @@ def get_num_commits(user, start_date, token) -> bool:
      """
     count = 0
     data = run_graphql_query(query, variables, token)
-    for repo in data['organization']['teams']['nodes'][0]['repositories']['nodes']:
-        count += int(repo['ref']['target']['history']['totalCount'])
+    for repo in data["organization"]["teams"]["nodes"][0]["repositories"]["nodes"]:
+        count += int(repo["ref"]["target"]["history"]["totalCount"])
         if count >= User.THRESHOLD:
             break
     return count
 
+
 def is_new_committer_query_user(user, start_date, token):
     user_query = """
         query {
@@ -185,9 +198,11 @@ def is_new_committer_query_user(user, start_date, token):
     data = run_graphql_query(user_query, {}, token)
     variables = {
         # We can only check one year of date at a time, so check for contribution between 3 and 4 years ago.
-        "start_date" : (start_date - datetime.timedelta(weeks = 2 * 52)).strftime("%Y-%m-%dT%H:%M:%S"),
-        "org" : data['organization']['id'],
-        "user" : user
+        "start_date": (start_date - datetime.timedelta(weeks=2 * 52)).strftime(
+            "%Y-%m-%dT%H:%M:%S"
+        ),
+        "org": data["organization"]["id"],
+        "user": user,
     }
     query = """
         query ($user: String!, $start_date: DateTime!, $org:ID!){
@@ -200,13 +215,14 @@ def is_new_committer_query_user(user, start_date, token):
     """
 
     data = run_graphql_query(query, variables, token)
-    if int(data['user']['contributionsCollection']['totalCommitContributions']) > 0:
+    if int(data["user"]["contributionsCollection"]["totalCommitContributions"]) > 0:
         return False
     return True
 
+
 def is_new_committer_query_repo(user, start_date, token):
     variables = {
-        'user' : user,
+        "user": user,
     }
 
     user_query = """
@@ -218,9 +234,9 @@ def is_new_committer_query_repo(user, start_date, token):
     """
 
     data = run_graphql_query(user_query, variables, token)
-    variables['owner'] = 'llvm'
-    variables['user_id'] = data['user']['id']
-    variables['start_date'] = start_date.strftime("%Y-%m-%dT%H:%M:%S")
+    variables["owner"] = "llvm"
+    variables["user_id"] = data["user"]["id"]
+    variables["start_date"] = start_date.strftime("%Y-%m-%dT%H:%M:%S")
 
     query = """
         query ($owner: String!, $user_id: ID!){
@@ -243,15 +259,16 @@ def is_new_committer_query_repo(user, start_date, token):
      """
 
     data = run_graphql_query(query, variables, token)
-    repo = data['organization']['repository']
-    commits = repo['ref']['target']['history']['nodes']
+    repo = data["organization"]["repository"]
+    commits = repo["ref"]["target"]["history"]["nodes"]
     if len(commits) == 0:
         return True
-    committed_date = commits[-1]['committedDate']
+    committed_date = commits[-1]["committedDate"]
     if datetime.datetime.strptime(committed_date, "%Y-%m-%dT%H:%M:%SZ") < start_date:
         return False
     return True
 
+
 def is_new_committer_pr_author(user, start_date, token):
     query = """
         query ($query: String!) {
@@ -262,14 +279,14 @@ def is_new_committer_pr_author(user, start_date, token):
         """
     formatted_start_date = start_date.strftime("%Y-%m-%dT%H:%M:%S")
     variables = {
-        "owner" : "llvm",
-        "repo" : "llvm-project",
-        "user" : user,
-        "query" : f"type:pr author:{user} created:>{formatted_start_date} org:llvm"
+        "owner": "llvm",
+        "repo": "llvm-project",
+        "user": user,
+        "query": f"type:pr author:{user} created:>{formatted_start_date} org:llvm",
     }
 
     data = run_graphql_query(query, variables, token)
-    return int(data['search']['issueCount']) > 0
+    return int(data["search"]["issueCount"]) > 0
 
 
 def is_new_committer(user, start_date, token):
@@ -279,8 +296,8 @@ def is_new_committer(user, start_date, token):
         pass
     return True
 
-def get_review_count(user, start_date, token):
 
+def get_review_count(user, start_date, token):
     query = """
         query ($query: String!) {
           search(query: $query, type: ISSUE, first: 5) {
@@ -290,14 +307,14 @@ def get_review_count(user, start_date, token):
         """
     formatted_start_date = start_date.strftime("%Y-%m-%dT%H:%M:%S")
     variables = {
-        "owner" : "llvm",
-        "repo" : "llvm-project",
-        "user" : user,
-        "query" : f"type:pr commenter:{user} -author:{user} merged:>{formatted_start_date} org:llvm"
+        "owner": "llvm",
+        "repo": "llvm-project",
+        "user": user,
+        "query": f"type:pr commenter:{user} -author:{user} merged:>{formatted_start_date} org:llvm",
     }
 
     data = run_graphql_query(query, variables, token)
-    return int(data['search']['issueCount'])
+    return int(data["search"]["issueCount"])
 
 
 def count_prs(triage_list, start_date, token):
@@ -325,47 +342,46 @@ def count_prs(triage_list, start_date, token):
     date_begin = start_date
     date_end = None
     while date_begin < datetime.datetime.now():
-        date_end = date_begin + datetime.timedelta(days = 7)
+        date_end = date_begin + datetime.timedelta(days=7)
         formatted_date_begin = date_begin.strftime("%Y-%m-%dT%H:%M:%S")
         formatted_date_end = date_end.strftime("%Y-%m-%dT%H:%M:%S")
         variables = {
-          "query" : f"type:pr is:merged merged:{formatted_date_begin}..{formatted_date_end} org:llvm",
+            "query": f"type:pr is:merged merged:{formatted_date_begin}..{formatted_date_end} org:llvm",
         }
         has_next_page = True
         while has_next_page:
             print(variables)
             data = run_graphql_query(query, variables, token)
-            for pr in data['search']['nodes']:
+            for pr in data["search"]["nodes"]:
                 # Users can be None if the user has been deleted.
-                if not pr['author']:
+                if not pr["author"]:
                     continue
-                author = pr['author']['login']
+                author = pr["author"]["login"]
                 if author in triage_list:
                     triage_list[author].add_authored()
 
-                if not pr['mergedBy']:
+                if not pr["mergedBy"]:
                     continue
-                merger = pr['mergedBy']['login']
+                merger = pr["mergedBy"]["login"]
                 if author == merger:
                     continue
                 if merger not in triage_list:
                     continue
                 triage_list[merger].add_merged()
 
-            has_next_page = data['search']['pageInfo']['hasNextPage']
+            has_next_page = data["search"]["pageInfo"]["hasNextPage"]
             if has_next_page:
-                variables['after'] = data['search']['pageInfo']['endCursor']
+                variables["after"] = data["search"]["pageInfo"]["endCursor"]
         date_begin = date_end
 
 
 def main():
-
     token = sys.argv[1]
     gh = github.Github(login_or_token=token)
-    org = gh.get_organization('llvm')
-    repo = org.get_repo('llvm-project')
-    team = org.get_team_by_slug('llvm-committers')
-    one_year_ago = datetime.datetime.now() - datetime.timedelta(days = 365)
+    org = gh.get_organization("llvm")
+    repo = org.get_repo("llvm-project")
+    team = org.get_team_by_slug("llvm-committers")
+    one_year_ago = datetime.datetime.now() - datetime.timedelta(days=365)
     triage_list = {}
     for member in team.get_members():
         triage_list[member.login] = User(member.login, triage_list)
@@ -408,15 +424,15 @@ def main():
 
     # Step 4 check for new committers
     for user in list(triage_list.keys()):
-        print ("Checking", user)
+        print("Checking", user)
         if is_new_committer(user, one_year_ago, token):
             print("Removing new committer: ", user)
             del triage_list[user]
 
     print("Complete:", len(triage_list), "triagers")
 
-    filename = 'triagers.log'
-    f = open(filename, 'w')
+    filename = "triagers.log"
+    f = open(filename, "w")
     for user in triage_list:
         print(triage_list[user].__repr__())
         f.write(user + "\n")

>From f10b5270db31e4dca03f0d57e30c25a3a97b8fa5 Mon Sep 17 00:00:00 2001
From: Tom Stellard <tstellar at redhat.com>
Date: Wed, 29 May 2024 10:08:17 -0700
Subject: [PATCH 4/7] Clarify triage role rules

---
 llvm/docs/DeveloperPolicy.rst | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/llvm/docs/DeveloperPolicy.rst b/llvm/docs/DeveloperPolicy.rst
index 43e1141edb82d..1a7bbd1a19225 100644
--- a/llvm/docs/DeveloperPolicy.rst
+++ b/llvm/docs/DeveloperPolicy.rst
@@ -536,9 +536,10 @@ project and don't need commit access, you may request triage access instead
 of commit access when you email Chris.
 
 Also, once a month we review the list of users with commit access and ask anyone
-(via a GitHub issue) with less than 5 interactions (i.e commits, created PRs, or
-PR comments) if they  still need commit access.  If a user does not respond within
-30 days, then they will be moved from the Write Role to the Triage Role.
+(via a GitHub issue) with less than 5 interactions in the last year (i.e commits,
+created PRs, or PR comments) if they  still need commit access.  If a user does
+not respond within 30 days, then they will be moved from the Write Role to the
+Triage Role.
 
 
 .. _discuss the change/gather consensus:

>From 1072de34173e973869a74df190a26bf566dfb816 Mon Sep 17 00:00:00 2001
From: Tom Stellard <tstellar at redhat.com>
Date: Thu, 18 Jul 2024 12:56:01 -0700
Subject: [PATCH 5/7] Review feedback

---
 .github/workflows/commit-access-review.py | 25 ++++++++++++++---------
 llvm/docs/DeveloperPolicy.rst             | 15 --------------
 2 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/.github/workflows/commit-access-review.py b/.github/workflows/commit-access-review.py
index 3a01ab570f4a1..e81d440ccc37d 100644
--- a/.github/workflows/commit-access-review.py
+++ b/.github/workflows/commit-access-review.py
@@ -1,3 +1,14 @@
+#!/usr/bin/env python3
+# ===-- commit-access-review.py  --------------------------------------------===#
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+# ===------------------------------------------------------------------------===#
+#
+# ===------------------------------------------------------------------------===#
+
 import datetime
 import github
 import re
@@ -129,9 +140,6 @@ def check_manual_requests(start_date, token) -> bool:
     users = []
     for issue in data["search"]["nodes"]:
         users.extend([user[1:] for user in re.findall("@[^ ,\n]+", issue["body"])])
-        # Do we need to check comments if we are checking mentions??
-        # for comment in issue['comments']['nodes']:
-        #    users.append(comment['author']['login'])
 
     return users
 
@@ -388,7 +396,6 @@ def main():
 
     print("Start:", len(triage_list), "triagers")
     # Step 0 Check if users have requested commit access in the last year.
-
     for user in check_manual_requests(one_year_ago, token):
         if user in triage_list:
             print(user, "requested commit access in the last year.")
@@ -431,12 +438,10 @@ def main():
 
     print("Complete:", len(triage_list), "triagers")
 
-    filename = "triagers.log"
-    f = open(filename, "w")
-    for user in triage_list:
-        print(triage_list[user].__repr__())
-        f.write(user + "\n")
-    f.close()
+    with open("triagers.log", "w") as triagers_log:
+        for user in triage_list:
+            print(triage_list[user].__repr__())
+            triagers_log.write(user + "\n")
 
 
 if __name__ == "__main__":
diff --git a/llvm/docs/DeveloperPolicy.rst b/llvm/docs/DeveloperPolicy.rst
index 1a7bbd1a19225..f2ac46e6c04f2 100644
--- a/llvm/docs/DeveloperPolicy.rst
+++ b/llvm/docs/DeveloperPolicy.rst
@@ -527,21 +527,6 @@ after they are committed, depending on the nature of the change).  You are
 encouraged to review other peoples' patches as well, but you aren't required
 to do so.
 
-Triage Role
-^^^^^^^^^^^^^
-
-The `Triage Role <https://docs.github.com/en/organizations/managing-user-access-to-your-organizations-repositories/managing-repository-roles/repository-roles-for-an-organization>`_ in GitHub gives users elevated permissions for managing issues and
-pull requests.  If you want to help triage issues and pull requests in the
-project and don't need commit access, you may request triage access instead
-of commit access when you email Chris.
-
-Also, once a month we review the list of users with commit access and ask anyone
-(via a GitHub issue) with less than 5 interactions in the last year (i.e commits,
-created PRs, or PR comments) if they  still need commit access.  If a user does
-not respond within 30 days, then they will be moved from the Write Role to the
-Triage Role.
-
-
 .. _discuss the change/gather consensus:
 
 Making a Major Change

>From 4b535813fb55cf63a0dbb465ebb51fe72b7ab337 Mon Sep 17 00:00:00 2001
From: Tom Stellard <tstellar at redhat.com>
Date: Thu, 18 Jul 2024 13:27:53 -0700
Subject: [PATCH 6/7] Add annotations/doc strings

---
 .github/workflows/commit-access-review.py | 84 +++++++----------------
 1 file changed, 26 insertions(+), 58 deletions(-)

diff --git a/.github/workflows/commit-access-review.py b/.github/workflows/commit-access-review.py
index e81d440ccc37d..0297eb801de08 100644
--- a/.github/workflows/commit-access-review.py
+++ b/.github/workflows/commit-access-review.py
@@ -112,7 +112,11 @@ def run_graphql_query(
         )
 
 
-def check_manual_requests(start_date, token) -> bool:
+def check_manual_requests(start_date: datetime.datetime, token: str) -> list[str]:
+    """
+    Return a list of users who have been asked since ``start_date`` if they
+    want to keep their commit access.
+    """
     query = """
         query ($query: String!) {
           search(query: $query, type: ISSUE, first: 100) {
@@ -144,7 +148,10 @@ def check_manual_requests(start_date, token) -> bool:
     return users
 
 
-def get_num_commits(user, start_date, token) -> bool:
+def get_num_commits(user: str, start_date: datetime.datetime, token: str) -> int:
+    """
+    Get number of commits that ``user`` has been made since ``start_date`.
+    """
     variables = {
         "owner": "llvm",
         "user": user,
@@ -194,41 +201,11 @@ def get_num_commits(user, start_date, token) -> bool:
     return count
 
 
-def is_new_committer_query_user(user, start_date, token):
-    user_query = """
-        query {
-          organization(login: "llvm") {
-            id
-          }
-        }
+def is_new_committer_query_repo(user: str, start_date: datetime.datetime, token: str) -> bool:
     """
-
-    data = run_graphql_query(user_query, {}, token)
-    variables = {
-        # We can only check one year of date at a time, so check for contribution between 3 and 4 years ago.
-        "start_date": (start_date - datetime.timedelta(weeks=2 * 52)).strftime(
-            "%Y-%m-%dT%H:%M:%S"
-        ),
-        "org": data["organization"]["id"],
-        "user": user,
-    }
-    query = """
-        query ($user: String!, $start_date: DateTime!, $org:ID!){
-          user(login: $user) {
-            contributionsCollection(from:$start_date, organizationID:$org) {
-                totalCommitContributions
-            }
-          }
-        }
+    Determine if ``user`` is a new committer.  A new committer can keep their
+    commit access even if they don't meet the criteria.
     """
-
-    data = run_graphql_query(query, variables, token)
-    if int(data["user"]["contributionsCollection"]["totalCommitContributions"]) > 0:
-        return False
-    return True
-
-
-def is_new_committer_query_repo(user, start_date, token):
     variables = {
         "user": user,
     }
@@ -277,27 +254,10 @@ def is_new_committer_query_repo(user, start_date, token):
     return True
 
 
-def is_new_committer_pr_author(user, start_date, token):
-    query = """
-        query ($query: String!) {
-          search(query: $query, type: ISSUE, first: 5) {
-            issueCount
-          }
-        }
-        """
-    formatted_start_date = start_date.strftime("%Y-%m-%dT%H:%M:%S")
-    variables = {
-        "owner": "llvm",
-        "repo": "llvm-project",
-        "user": user,
-        "query": f"type:pr author:{user} created:>{formatted_start_date} org:llvm",
-    }
-
-    data = run_graphql_query(query, variables, token)
-    return int(data["search"]["issueCount"]) > 0
-
-
-def is_new_committer(user, start_date, token):
+def is_new_committer(user: str, start_date: datetime.datetime, token: str) -> bool:
+    """
+    Wrapper around is_new_commiter_query_repo to handle exceptions.
+    """
     try:
         return is_new_committer_query_repo(user, start_date, token)
     except:
@@ -305,7 +265,10 @@ def is_new_committer(user, start_date, token):
     return True
 
 
-def get_review_count(user, start_date, token):
+def get_review_count(user: str, start_date: datetime.datetime, token: str) -> int:
+    """
+    Return the number of reviews that ``user`` has done since ``start_date``. 
+    """
     query = """
         query ($query: String!) {
           search(query: $query, type: ISSUE, first: 5) {
@@ -325,7 +288,12 @@ def get_review_count(user, start_date, token):
     return int(data["search"]["issueCount"])
 
 
-def count_prs(triage_list, start_date, token):
+def count_prs(triage_list: dict, start_date: datetime.datetime, token: str):
+    """
+    Fetch all the merged PRs for the project since ``start_date`` and update
+    ``triage_list`` with the number of PRs merged for each user.
+    """
+
     query = """
         query ($query: String!, $after: String) {
           search(query: $query, type: ISSUE, first: 100, after: $after) {

>From cc9496c4f10f9ad14c56c5a18a24cd2a65dc6e70 Mon Sep 17 00:00:00 2001
From: Tom Stellard <tstellar at redhat.com>
Date: Thu, 18 Jul 2024 13:39:14 -0700
Subject: [PATCH 7/7] Fix python formatting

---
 .github/workflows/commit-access-review.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/commit-access-review.py b/.github/workflows/commit-access-review.py
index 0297eb801de08..97589c138c0b7 100644
--- a/.github/workflows/commit-access-review.py
+++ b/.github/workflows/commit-access-review.py
@@ -201,7 +201,9 @@ def get_num_commits(user: str, start_date: datetime.datetime, token: str) -> int
     return count
 
 
-def is_new_committer_query_repo(user: str, start_date: datetime.datetime, token: str) -> bool:
+def is_new_committer_query_repo(
+    user: str, start_date: datetime.datetime, token: str
+) -> bool:
     """
     Determine if ``user`` is a new committer.  A new committer can keep their
     commit access even if they don't meet the criteria.
@@ -267,7 +269,7 @@ def is_new_committer(user: str, start_date: datetime.datetime, token: str) -> bo
 
 def get_review_count(user: str, start_date: datetime.datetime, token: str) -> int:
     """
-    Return the number of reviews that ``user`` has done since ``start_date``. 
+    Return the number of reviews that ``user`` has done since ``start_date``.
     """
     query = """
         query ($query: String!) {



More information about the llvm-commits mailing list