From 0725e99d3e9ee8536cdfdcb539912613d379d933 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Tue, 8 Aug 2023 12:34:59 -0400 Subject: [PATCH] feat!: removed any knowledge of who is a core contributor. #227 --- changelog.d/20230808_123250_nedbat_no_cc.rst | 6 ++ docs/details.rst | 14 +---- openedx_webhooks/bot_comments.py | 19 ------ openedx_webhooks/info.py | 32 ---------- openedx_webhooks/labels.py | 1 - openedx_webhooks/tasks/pr_tracking.py | 12 ---- .../github_committer_pr_comment.md.j2 | 24 -------- tests/test_info.py | 41 +------------ tests/test_pull_request_opened.py | 60 +------------------ 9 files changed, 9 insertions(+), 200 deletions(-) create mode 100644 changelog.d/20230808_123250_nedbat_no_cc.rst delete mode 100644 openedx_webhooks/templates/github_committer_pr_comment.md.j2 diff --git a/changelog.d/20230808_123250_nedbat_no_cc.rst b/changelog.d/20230808_123250_nedbat_no_cc.rst new file mode 100644 index 00000000..eb62de30 --- /dev/null +++ b/changelog.d/20230808_123250_nedbat_no_cc.rst @@ -0,0 +1,6 @@ +.. A new scriv changelog fragment. + +- Removed: any understanding of who is a core contributor. The bot no longer + makes comments or labels particular to core contributors. `Issue 227`_. + +.. _issue 227: https://github.com/openedx/openedx-webhooks/issues/227 diff --git a/docs/details.rst b/docs/details.rst index bf391e25..0b865ce1 100644 --- a/docs/details.rst +++ b/docs/details.rst @@ -35,9 +35,6 @@ handled: - If the title of the pull request indicates this is a blended project (with "[BD-XXX]" in the title), then this is a blended pull request. -- The author is checked for core committer status in the repo. If so, this is - a core commiter pull request. - - Otherwise, this is a regular pull request. Additionally, if the pull request is in draft status, or has "WIP" in the @@ -68,8 +65,6 @@ Now we can decide what to do: - Initial Jira status: - - Core committer pull requests get "Waiting on Author". - - Blended pull requests get "Needs Triage". - For regular pull requests, if the author doesn't have a signed CLA, the @@ -87,17 +82,13 @@ be changed. - Blended pull requests get "blended" applied as a GitHub label and Jira label. - - Core committer pull requests get "core-commiter" as a Jira label and - "open-source-contribution" as a GitHub label. - - Regular pull requests just get "open-source-contribution" as a GitHub label. - The initial Jira status is set as a GitHub label on the pull request. - Initial bot comment: - - Each kind of pull request (blended, core committer, and regular) gets - different comments. + - Each kind of pull request (blended and regular) gets different comments. - If the user doesn't have a signed CLA, the bot adds a paragraph about needing to sign one. @@ -146,9 +137,6 @@ When a pull request is closed The bot leaves a comment asking the author to complete a survey about the pull request. -If the pull request was a core committer PR, the bot leaves a comment pinging -the committer's edX champions, to help them stay current. - When a pull request is re-opened -------------------------------- diff --git a/openedx_webhooks/bot_comments.py b/openedx_webhooks/bot_comments.py index 6dac420e..14b4edeb 100644 --- a/openedx_webhooks/bot_comments.py +++ b/openedx_webhooks/bot_comments.py @@ -28,7 +28,6 @@ class BotComment(Enum): WELCOME = auto() WELCOME_CLOSED = auto() NEED_CLA = auto() - CORE_COMMITTER = auto() BLENDED = auto() OK_TO_TEST = auto() END_OF_WIP = auto() @@ -48,9 +47,6 @@ class BotComment(Enum): "", "We can't start reviewing your pull request until you've submitted", ], - BotComment.CORE_COMMITTER: [ - "", - ], BotComment.BLENDED: [ "", ], @@ -76,7 +72,6 @@ class BotComment(Enum): BotComment.WELCOME_CLOSED, BotComment.NEED_CLA, BotComment.BLENDED, - BotComment.CORE_COMMITTER, BotComment.OK_TO_TEST, BotComment.END_OF_WIP, BotComment.NO_CONTRIBUTIONS, @@ -124,20 +119,6 @@ def github_community_pr_comment_closed(pull_request: PrDict, issue_key: str, **k ) -def github_committer_pr_comment(pull_request: PrDict, issue_key: str, **kwargs) -> str: - """ - Create the body of the comment for new pull requests from core committers. - """ - return render_template( - "github_committer_pr_comment.md.j2", - user=pull_request["user"]["login"], - issue_key=issue_key, - is_draft=is_draft_pull_request(pull_request), - jira_server=settings.JIRA_SERVER, - **kwargs - ) - - def github_blended_pr_comment( pull_request: PrDict, issue_key: str, diff --git a/openedx_webhooks/info.py b/openedx_webhooks/info.py index ca8b6719..5c16f46a 100644 --- a/openedx_webhooks/info.py +++ b/openedx_webhooks/info.py @@ -204,38 +204,6 @@ def _pr_author_data(pull_request: PrDict) -> Optional[Dict]: return people.get(author) -def is_committer_pull_request(pull_request: PrDict) -> bool: - """ - Was this pull request created by a core committer for this repo - or branch? - """ - person = _pr_author_data(pull_request) - if person is None: - return False - if "committer" not in person: - return False - - repo = pull_request["base"]["repo"]["full_name"] - org = repo.partition("/")[0] - branch = pull_request["base"]["ref"] - commit_rights = person["committer"] - if not commit_rights: - return False - if "orgs" in commit_rights: - if org in commit_rights["orgs"]: - return True - if "repos" in commit_rights: - if repo in commit_rights["repos"]: - return True - if "branches" in commit_rights: - for access_branch in commit_rights["branches"]: - if access_branch.endswith("*") and branch.startswith(access_branch[:-1]): - return True - elif branch == access_branch: - return True - return False - - NO_CONTRIBUTION_ORGS = {"edx"} def repo_refuses_contributions(pull_request: PrDict) -> bool: diff --git a/openedx_webhooks/labels.py b/openedx_webhooks/labels.py index d16c95bf..1fd72823 100644 --- a/openedx_webhooks/labels.py +++ b/openedx_webhooks/labels.py @@ -19,5 +19,4 @@ JIRA_CATEGORY_LABELS = { "blended", - "core-committer", } diff --git a/openedx_webhooks/tasks/pr_tracking.py b/openedx_webhooks/tasks/pr_tracking.py index e4c944e1..24e56382 100644 --- a/openedx_webhooks/tasks/pr_tracking.py +++ b/openedx_webhooks/tasks/pr_tracking.py @@ -18,7 +18,6 @@ extract_data_from_comment, format_data_for_comment, github_blended_pr_comment, - github_committer_pr_comment, github_community_pr_comment, github_community_pr_comment_closed, github_end_survey_comment, @@ -43,7 +42,6 @@ get_jira_issue_key, get_people_file, is_bot_pull_request, - is_committer_pull_request, is_draft_pull_request, is_internal_pull_request, is_private_repo_no_cla_pull_request, @@ -292,12 +290,6 @@ def desired_support_state(pr: PrDict) -> Optional[PrDesiredInfo]: comment = BotComment.WELCOME_CLOSED desired.jira_project = jira_project_for_ospr(pr) desired.github_labels.add("open-source-contribution") - committer = is_committer_pull_request(pr) - if committer: - comment = BotComment.CORE_COMMITTER - desired.jira_labels.add("core-committer") - desired.jira_initial_status = "Waiting on Author" - desired.bot_comments.add(BotComment.CORE_COMMITTER) desired.bot_comments.add(comment) assert settings.GITHUB_OSPR_PROJECT, "You must set GITHUB_OSPR_PROJECT" @@ -616,10 +608,6 @@ def _fix_bot_comment(self, comment_kwargs: Dict) -> None: if BotComment.SURVEY in self.desired.bot_comments: self.desired.bot_comments.remove(BotComment.SURVEY) - if BotComment.CORE_COMMITTER in needed_comments: - comment_body += github_committer_pr_comment(self.pr, jira_id, **comment_kwargs) - needed_comments.remove(BotComment.CORE_COMMITTER) - if BotComment.BLENDED in needed_comments: comment_body += github_blended_pr_comment( self.pr, diff --git a/openedx_webhooks/templates/github_committer_pr_comment.md.j2 b/openedx_webhooks/templates/github_committer_pr_comment.md.j2 deleted file mode 100644 index fbef2044..00000000 --- a/openedx_webhooks/templates/github_committer_pr_comment.md.j2 +++ /dev/null @@ -1,24 +0,0 @@ -{% filter replace("\n", " ")|trim %} -Thanks for the pull request, @{{ user }}! -{% if issue_key %} -I've created [{{ issue_key }}]({{ jira_server }}/browse/{{ issue_key }}) -to keep track of it in JIRA. -{% endif %} -{% endfilter %} - -{% filter replace("\n", " ")|trim %} -As a core committer in this repo, you can merge this once the pull request is approved per the -[core committer reviewer requirements](https://openedx.atlassian.net/wiki/spaces/COMM/pages/1529675973/Open+edX+Core+Committers#Reviews-and-Ownership) -and according to the agreement with your edX Champion. -{% endfilter %} - -{% if is_draft %} -{% filter replace("\n", " ")|trim %} -This is currently a draft pull request. When it is ready for our review and all tests are green, -click "Ready for Review", or remove "WIP" from the title, as appropriate. -{% endfilter %} - - -{% endif %} - - diff --git a/tests/test_info.py b/tests/test_info.py index f26fa34e..1a062a22 100644 --- a/tests/test_info.py +++ b/tests/test_info.py @@ -6,7 +6,7 @@ from openedx_webhooks.info import ( get_people_file, - is_committer_pull_request, is_internal_pull_request, is_draft_pull_request, + is_internal_pull_request, is_draft_pull_request, get_blended_project_id, ) @@ -59,45 +59,6 @@ def test_left_but_still_a_fan(make_pull_request): pr = make_pull_request("jarv") assert not is_internal_pull_request(pr) -def test_org_committers(make_pull_request): - pr = make_pull_request("felipemontoya", repo="openedx/something") - assert not is_internal_pull_request(pr) - assert is_committer_pull_request(pr) - pr = make_pull_request("felipemontoya", repo="edx/something") - assert not is_internal_pull_request(pr) - assert not is_committer_pull_request(pr) - -def test_repo_committers(make_pull_request): - pr = make_pull_request("pdpinch", repo="openedx/ccx-keys") - assert not is_internal_pull_request(pr) - assert is_committer_pull_request(pr) - pr = make_pull_request("pdpinch", repo="openedx/edx-platform") - assert not is_internal_pull_request(pr) - assert not is_committer_pull_request(pr) - -def test_base_branch_committers(make_pull_request): - pr = make_pull_request( - "hollyhunter", - repo="openedx/fake-release-repo", - ref="open-release/birch.1" - ) - assert not is_internal_pull_request(pr) - assert is_committer_pull_request(pr) - pr = make_pull_request( - "hollyhunter", - repo="openedx/fake-release-repo", - ref="master" - ) - assert not is_internal_pull_request(pr) - assert not is_committer_pull_request(pr) - pr = make_pull_request( - "pdpinch", - repo="openedx/fake-release-repo", - ref="open-release/birch.1" - ) - assert not is_internal_pull_request(pr) - assert not is_committer_pull_request(pr) - def test_current_person_no_institution(): people = get_people_file() current_person = people["jarv"] diff --git a/tests/test_pull_request_opened.py b/tests/test_pull_request_opened.py index 292834f6..9156ba2f 100644 --- a/tests/test_pull_request_opened.py +++ b/tests/test_pull_request_opened.py @@ -244,57 +244,6 @@ def test_psycho_reopening(fake_github, fake_jira): assert issue.status == status -def test_core_committer_pr_opened(has_jira, fake_github, fake_jira): - pr = fake_github.make_pull_request(user="felipemontoya", owner="openedx", repo="edx-platform") - prj = pr.as_json() - - issue_id, anything_happened = pull_request_changed(prj) - - assert anything_happened is True - if has_jira: - assert issue_id is not None - assert issue_id.startswith("OSPR-") - - # Check the Jira issue that was created. - assert len(fake_jira.issues) == 1 - issue = fake_jira.issues[issue_id] - assert issue.contributor_name == "Felipe Montoya" - assert issue.customer == ["EduNEXT"] - assert issue.pr_number == prj["number"] - assert issue.repo == prj["base"]["repo"]["full_name"] - assert issue.url == prj["html_url"] - assert issue.description == prj["body"] - assert issue.issuetype == "Pull Request Review" - assert issue.summary == prj["title"] - assert issue.labels == {"core-committer"} - - # Check that the Jira issue was moved to Waiting on Author - assert issue.status == "Waiting on Author" - else: - assert issue_id is None - assert len(fake_jira.issues) == 0 - - # Check the GitHub comment that was created. - pr_comments = pr.list_comments() - assert len(pr_comments) == 1 - body = pr_comments[0].body - check_issue_link_in_markdown(body, issue_id) - assert "Thanks for the pull request, @felipemontoya!" in body - assert is_comment_kind(BotComment.CORE_COMMITTER, body) - assert not is_comment_kind(BotComment.NEED_CLA, body) - assert is_comment_kind(BotComment.OK_TO_TEST, body) - - # Check the GitHub labels that got applied. - expected_labels = {"open-source-contribution"} - if has_jira: - expected_labels.add("waiting on author") - assert pr.labels == expected_labels - # Check the status check applied to the latest commit. - assert pr.status(CLA_CONTEXT) == CLA_STATUS_GOOD - # It should have been put in the OSPR project. - assert pull_request_projects(pr.as_json()) == {settings.GITHUB_OSPR_PROJECT} - - EXAMPLE_PLATFORM_MAP_1_2 = { "child": { "id": "14522", @@ -617,7 +566,7 @@ def test_title_change_but_issue_already_moved(fake_github, fake_jira): assert get_jira_issue_key(prj) == (True, issue_id) -@pytest.mark.parametrize("pr_type", ["normal", "blended", "committer", "nocla"]) +@pytest.mark.parametrize("pr_type", ["normal", "blended", "nocla"]) @pytest.mark.parametrize("jira_got_fiddled", [ pytest.param(False, id="jira:notfiddled"), pytest.param(True, id="jira:fiddled"), @@ -646,9 +595,6 @@ def test_draft_pr_opened(pr_type, jira_got_fiddled, has_jira, fake_github, fake_ title2 = "[BD-34] Something good" initial_status = "Needs Triage" pr = fake_github.make_pull_request(user="tusbar", title=title1) - elif pr_type == "committer": - initial_status = "Waiting on Author" - pr = fake_github.make_pull_request(user="felipemontoya", owner="openedx", repo="edx-platform", title=title1) else: assert pr_type == "nocla" initial_status = "Community Manager Review" @@ -672,8 +618,6 @@ def test_draft_pr_opened(pr_type, jira_got_fiddled, has_jira, fake_github, fake_ assert issue.labels == set() elif pr_type == "blended": assert issue.labels == {"blended"} - elif pr_type == "committer": - assert issue.labels == {"core-committer"} else: assert pr_type == "nocla" assert issue.labels == set() @@ -703,8 +647,6 @@ def test_draft_pr_opened(pr_type, jira_got_fiddled, has_jira, fake_github, fake_ assert is_comment_kind(BotComment.WELCOME, body) elif pr_type == "blended": assert is_comment_kind(BotComment.BLENDED, body) - elif pr_type == "committer": - assert is_comment_kind(BotComment.CORE_COMMITTER, body) else: assert pr_type == "nocla" assert is_comment_kind(BotComment.NEED_CLA, body)