Add branch protection support for required_status_checks.checks object
#2884
Conversation
|
Hi @EnricoMi could you please have a look at the coding if this is the right direction and what needs to be adjusted. Thanks! |
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2884 +/- ##
==========================================
- Coverage 96.70% 96.64% -0.07%
==========================================
Files 147 147
Lines 14885 14887 +2
==========================================
- Hits 14395 14388 -7
- Misses 490 499 +9 ☔ View full report in Codecov by Sentry. |
github/Branch.py
Outdated
| ) -> RequiredStatusChecks: | ||
| """ | ||
| :calls: `PATCH /repos/{owner}/{repo}/branches/{branch}/protection/required_status_checks <https://docs.github.com/en/rest/reference/repos#branches>`_ | ||
| """ | ||
| assert is_optional(strict, bool), strict | ||
| assert is_optional_list(contexts, str), contexts | ||
| # assert is_optional_list(checks, Union[str, Tuple[str, int]]), checks |
There was a problem hiding this comment.
| # assert is_optional_list(checks, Union[str, Tuple[str, int]]), checks | |
| assert is_optional_list(checks, (str, tuple)), checks | |
| if checks is not None: | |
| assert all(not isinstance(check, tuple) or list(map(type, check)) == [str, int] for check in checks), checks |
There was a problem hiding this comment.
thanks, have done the if checks is not None: in another way because that way it didn't worked for None. Is this OK?
| elif is_defined(checks): | ||
| post_parameters["checks"] = [] | ||
| for check in checks: | ||
| if isinstance(check, tuple): | ||
| context, app_id = check | ||
| post_parameters["checks"].append({"context": context, "app_id": app_id}) | ||
| else: | ||
| post_parameters["checks"].append({"context": check}) | ||
| elif is_defined(contexts): | ||
| post_parameters["checks"] = [{"context": context} for context in contexts] |
There was a problem hiding this comment.
I think you want to populate list checks here, as used below to set post_parameters["required_status_checks"].
| {"checks": [{"context": "check1"}, {"context": "check2", "app_id": -1}, {"context": "check3", "app_id": 123456}]} | ||
| 200 | ||
| [('status', '200 OK'), ('x-ratelimit-remaining', '4994'), ('content-length', '0'), ('server', 'nginx/1.0.13'), ('connection', 'keep-alive'), ('x-ratelimit-limit', '5000'), ('etag', '"81ba94a48ad867b26d48c023ac584f43"'), ('date', 'Mon, 07 May 2018 13:42:41 GMT'), ('content-type', 'application/json; charset=utf-8')] | ||
| '' |
There was a problem hiding this comment.
looks like this is not correct
There was a problem hiding this comment.
how to create the file automatically? This does not work for me in macOS
pytest -k Branch.testEditRequiredStatusChecksChecks --record --auth_with_token
(pygithub-dev) <user>@<computer> PyGithub % pytest -k Branch.testEditRequiredStatusChecksChecks --record --auth_with_token
=============================================================================================== test session starts ===============================================================================================
platform darwin -- Python 3.10.13, pytest-7.4.4, pluggy-1.3.0
rootdir: /Users/<user>/VSCode/python/PyGithub
configfile: pyproject.toml
plugins: github-actions-annotate-failures-0.2.0, subtests-0.11.0, cov-4.1.0
collected 890 items / 890 deselected / 0 selected
============================================================================================= 890 deselected in 0.32s =============================================================================================
|
|
||
| post_parameters: dict[str, Any] = NotSet.remove_unset_items({"strict": strict, "contexts": contexts}) |
There was a problem hiding this comment.
please keep the NotSet.remove_unset_items pattern where it exists, preferred over is_defined + post_parameters = ... above.
There was a problem hiding this comment.
tried to comply with the pattern but didn't found a example in PyGithub which uses kind of calculation of the parameters like I do here and sets post_parameters afterwards.
I dont think 8776ae5 makes it easier to read, maybe you could make it clearer what I have to adjust based on that.
- inkl. replay data and adjusting the other test
- inkl. adjusting the test
treee111
force-pushed
the
req-status-checks-renewal
branch
from
8776ae5
to
f9f04cb
Compare
as outlined in #2873 (comment):
I've done:
checksas another argumentchecksshould be of typeList[Union[str, Tuple[str, int]]]so that this allows forchecks=["check1", ("check2", -1), ("check3", 123456)]contextis given, it's values should be used to populaterequired_status_checks.checks.context, rather thanrequired_status_checks.contextschecksandcontextare given,contextshould be ignoredone thing I am unsure about is how to check for
checks, this commented-out section is not working...https://github.com/treee111/PyGithub/blob/cce1d1de9795a07e2134ae1e780d6b922bd59d96/github/Branch.py#L349
I created the replay data manually because the command to create them automatically did not work for me.
closes #2157