Skip to content

Commit 5522f06

Browse files
authored
Disable review approval portion of downstream change check (#449)
The default access token used in GitHub Actions workflows does not have the required permissions to see the list of team members, which exists outside of the arm-toolchain repo at the arm org level. As a result, attempting to check which teams have reviewed a pull request will currently cause an error. For now, remove the check until it can be reworked. Since there will no longer be a hard gated check on whether a pull request has been sufficiently reviewed, the automated comment has been updated to remind to wait for both teams before merging.
1 parent a5639e2 commit 5522f06

File tree

2 files changed

+11
-88
lines changed

2 files changed

+11
-88
lines changed

.github/workflows/check_downstream_changes.yml

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,15 @@ on:
2323
- reopened
2424
- edited
2525
- synchronize
26-
# Trigger whenever a pull request is reviewed to check which
27-
# teams have approved.
28-
pull_request_review:
29-
types:
30-
- submitted
31-
- edited
32-
- dismissed
26+
branches:
27+
- arm-software
28+
- release/arm-software/**
3329

3430
jobs:
3531
check-downstream-changes:
3632
runs-on: ubuntu-24.04-arm
3733

38-
if: github.repository == 'arm/arm-toolchain' && (github.event.pull_request.base.ref == 'arm-software' || startsWith(github.event.pull_request.base.ref, 'release/arm-software/'))
34+
if: github.repository == 'arm/arm-toolchain'
3935

4036
steps:
4137
- name: Checkout

arm-software/ci/check_downstream_changes.py

Lines changed: 7 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@
99
A script to check that a pull request adheres to the downstream patch policy.
1010
If the pull request modifies file outside the arm-software build directory
1111
(or any other files excluded from automerge) then the pull request needs to
12-
contain specific text to link to a downstream tracking issue, and requires
13-
additional reviews.
12+
contain specific text to link to a downstream tracking issue.
1413
1514
Requires the GitHub CLI tool (gh) to query the repo.
1615
"""
@@ -30,10 +29,8 @@
3029
logger = logging.getLogger(__name__)
3130

3231
MERGE_IGNORE_PATHSPEC_FILE = Path(__file__).parent / ".automerge_ignore"
33-
ARM_ORG_NAME = "arm"
34-
REQUIRED_TEAM_APPROVALS = {"arm-toolchain-embedded", "arm-toolchain-linux"}
3532
HELP_COMMENT_TEXT = """This pull review modifies files outside of the `arm-software` directory, so please ensure it follows the [Downstream Patch Policy](https://github.com/arm/arm-toolchain/blob/arm-software/CONTRIBUTING.md#downstream-patch-policy).
36-
An automated check will test if the tagging requirements have been met. In addition, approved reviews from both Arm Toolchain for Embedded and Arm Toolchain for Linux teams will be required before this check will pass."""
33+
An automated check will test if the tagging requirements have been met. Please wait for approving reviews from both Arm Toolchain for Embedded and Arm Toolchain for Linux teams before merging."""
3734
DOWNSTREAM_CHANGE_LABEL = "downstream-change"
3835

3936

@@ -69,7 +66,7 @@ def get_pr_json(pr_num: str, repo: str) -> dict:
6966
"--repo",
7067
repo,
7168
"--json",
72-
"body,comments,files,labels,reviews,title",
69+
"body,comments,files,labels,title",
7370
]
7471
logger.debug(f"Running `{shlex.join(args)}`")
7572
try:
@@ -91,70 +88,6 @@ def get_pr_json(pr_num: str, repo: str) -> dict:
9188
return j
9289

9390

94-
# Use the api to get a list of members for a team.
95-
def get_team_members(org: str, team_name: str) -> set:
96-
args = [
97-
"gh",
98-
"api",
99-
"-H",
100-
"Accept: application/vnd.github+json",
101-
"-H",
102-
"X-GitHub-Api-Version: 2022-11-28",
103-
f"/orgs/{org}/teams/{team_name}/members",
104-
]
105-
logger.debug(f"Running `{shlex.join(args)}`")
106-
try:
107-
p = subprocess.run(
108-
args,
109-
check=True,
110-
capture_output=True,
111-
text=True,
112-
)
113-
except subprocess.CalledProcessError as error:
114-
logger.error(
115-
f"Check error. Failure fetching team members\ncmd:{shlex.join(error.cmd)}\ncode:{error.returncode}\nstdout:{error.stdout}\nstderr:{error.stderr}"
116-
)
117-
sys.exit(1)
118-
j = json.loads(p.stdout)
119-
logger.debug(
120-
f"Response from server for team {team_name}:\n{json.dumps(j, indent=4)}"
121-
)
122-
return j
123-
124-
125-
# Check that a pull review has approving reviews from a set of teams.
126-
def has_required_reviews(input_json: dict, teams_required: set[str]) -> bool:
127-
# Get the members of each time, and map to their logins.
128-
login_lookup = dict()
129-
for team_name in teams_required:
130-
team_json = get_team_members(ARM_ORG_NAME, team_name)
131-
for entry in team_json:
132-
# This assumes a user can't belong to both teams.
133-
login_lookup[entry["login"]] = team_name
134-
135-
# Get the corresponding team for each approved review.
136-
teams_remaining = teams_required.copy()
137-
for review in input_json["reviews"]:
138-
if review["state"] == "APPROVED":
139-
reviewer_login = review["author"]["login"]
140-
if reviewer_login in login_lookup:
141-
reviewer_team = login_lookup[reviewer_login]
142-
logger.info(
143-
f"Pull request has been approved by {reviewer_login} from the {reviewer_team} team."
144-
)
145-
if reviewer_team in teams_remaining:
146-
teams_remaining.remove(reviewer_team)
147-
148-
if len(teams_remaining) == 0:
149-
logger.info("Pull request has been reviewed by all required teams.")
150-
return True
151-
else:
152-
logger.info(
153-
f"Pull request has not been reviewed by all required teams, missing: {teams_remaining}."
154-
)
155-
return False
156-
157-
15891
# Check that a value matches a valid issue.
15992
def is_valid_issue_num(issue_num: str, repo: str) -> bool:
16093
args = [
@@ -392,16 +325,10 @@ def main():
392325
sys.exit(1)
393326
else:
394327
add_issue_label(args.pr, args.repo, pr_json)
395-
if has_required_reviews(pr_json, REQUIRED_TEAM_APPROVALS):
396-
logger.info(
397-
f"Check passed. Pull request #{args.pr} contains downstream changes, a correctly formatted link to a downstream tracking issue, and has been reviewed by both teams."
398-
)
399-
sys.exit(0)
400-
else:
401-
logger.info(
402-
f"Check failed. Pull request #{args.pr} contains downstream changes, and a correctly formatted link to a downstream tracking issue, but has not been reviewed by both teams."
403-
)
404-
sys.exit(1)
328+
logger.info(
329+
f"Check passed. Pull request #{args.pr} contains downstream changes, and a correctly formatted link to a downstream tracking issue."
330+
)
331+
sys.exit(0)
405332
else:
406333
if issue_num is None:
407334
logger.info(

0 commit comments

Comments
 (0)