feat!: removed any knowledge of who is a core contributor. #227

Author: Ned Batchelder

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

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.
@@ -143,9 +134,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
 --------------------------------

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()
     END_OF_WIP = auto()
     SURVEY = auto()
@@ -47,9 +46,6 @@ class BotComment(Enum):
         "<!-- comment:no_cla -->",
         "We can't start reviewing your pull request until you've submitted",
     ],
-    BotComment.CORE_COMMITTER: [
-        "<!-- comment:welcome-core-committer -->",
-    ],
     BotComment.BLENDED: [
         "<!-- comment:welcome-blended -->",
     ],
@@ -72,7 +68,6 @@ class BotComment(Enum):
     BotComment.WELCOME_CLOSED,
     BotComment.NEED_CLA,
     BotComment.BLENDED,
-    BotComment.CORE_COMMITTER,
     BotComment.END_OF_WIP,
     BotComment.NO_CONTRIBUTIONS,
 }
@@ -119,20 +114,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,

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:

openedx_webhooks/labels.py

@@ -19,5 +19,4 @@
 
 JIRA_CATEGORY_LABELS = {
     "blended",
-    "core-committer",
 }

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"
@@ -613,10 +605,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,

openedx_webhooks/templates/github_committer_pr_comment.md.j2

@@ -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"]

tests/test_info.py

@@ -242,56 +242,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)
-
-    # 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",
@@ -613,7 +563,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"),
@@ -642,9 +592,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"
@@ -668,8 +615,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()
@@ -699,8 +644,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)