Skip to content

Commit

Permalink
Handle new GitHub branch_protection API
Browse files Browse the repository at this point in the history
Fixes sigmavirus24#1112

Since ~Q1 2022, branch_protection has an additional "checks" field which deprecates "contexts":
"contexts": list[str]
"checks": list[TypedDict("checks", context=str, app_id=int | None)]

The GitHub API returns both and then errors if both are present on update.

So, when making unrelated changes, check whether both are present and if so remove the "contexts" field, on the assumption that "checks" contains all the same data plus possibly app_id fields that we should not overwrite with nil.

Does this look OK? Happy to add polish, tests etc. if it's the right approach.
  • Loading branch information
ecatmur committed Oct 7, 2022
1 parent 5e72d01 commit fa8acb7
Showing 1 changed file with 14 additions and 2 deletions.
16 changes: 14 additions & 2 deletions src/github3/repos/branch.py
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,10 @@ def update(
else None
),
}
if current_status["required_status_checks"] is not None and all(
key in current_status["required_status_checks"] for key in ("contexts", "checks")):
# Prefer the new "checks" field when updating
del current_status["required_status_checks"]["contexts"]
edit = {
"enabled": True,
"enforce_admins": (
Expand Down Expand Up @@ -1186,7 +1190,7 @@ def delete_contexts(self, contexts):
return self._boolean(resp, 204, 404)

@decorators.requires_auth
def update(self, strict=None, contexts=None):
def update(self, strict=None, contexts=None, checks=None):
"""Update required status checks for the branch.
This requires admin or owner permissions to the repository and
Expand All @@ -1201,6 +1205,10 @@ def update(self, strict=None, contexts=None):
Whether this should be strict protection or not.
:param list contexts:
A list of context names that should be required.
:param list checks:
A list of dicts describing status checks that should be required.
Keys are: "context": str, "app_id": int (optional).
Replaces "contexts".
:returns:
A new instance of this class with the updated information
:rtype:
Expand All @@ -1215,8 +1223,12 @@ def update(self, strict=None, contexts=None):
json = None
if strict is not None:
update_data["strict"] = strict
if contexts is not None:
if contexts is not None and checks is not None:
raise ValueError("cannot supply both 'contexts' and 'checks'")
elif contexts is not None:
update_data["contexts"] = contexts
elif checks is not None:
update_data["checks"] = checks
if update_data:
resp = self._patch(self.url, json=update_data)
json = self._json(resp, 200)
Expand Down

0 comments on commit fa8acb7

Please sign in to comment.