Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto add the skip news label to docs PRs #485

Merged
merged 13 commits into from
Jul 13, 2022
23 changes: 14 additions & 9 deletions bedevere/prtype.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ class Labels(enum.Enum):
performance = "performance"
type_security = f"{TYPE_LABEL_PREFIX}-security"
tests = "tests"
skip_news = "skip news"
ezio-melotti marked this conversation as resolved.
Show resolved Hide resolved


async def add_label(gh, issue, label):
"""Apply this type label if there aren't any type labels on the PR."""
if any(label.startswith("type") for label in util.labels(issue)):
return
await gh.post(issue["labels_url"], data=[label.value])
async def add_labels(gh, issue, labels):
"""Add the specified labels to the PR."""
current_labels = util.labels(issue)
label_names = [c.value for c in labels if c.value not in current_labels]
if label_names:
await gh.post(issue["labels_url"], data=label_names)


async def classify_by_filepaths(gh, pull_request, filenames):
Expand All @@ -35,10 +37,10 @@ async def classify_by_filepaths(gh, pull_request, filenames):
The routing is handled by the filepaths module.
"""
issue = await util.issue_for_PR(gh, pull_request)
docs = tests = False
news = docs = tests = False
for filename in filenames:
if util.is_news_dir(filename):
continue
news = True
filepath = pathlib.PurePath(filename)
if filepath.suffix == '.rst':
docs = True
Expand All @@ -47,7 +49,10 @@ async def classify_by_filepaths(gh, pull_request, filenames):
else:
return
if tests:
await add_label(gh, issue, Labels.tests)
await add_labels(gh, issue, [Labels.tests])
elif docs:
await add_label(gh, issue, Labels.docs)
if news:
await add_labels(gh, issue, [Labels.docs])
else:
await add_labels(gh, issue, [Labels.docs, Labels.skip_news])
return
6 changes: 5 additions & 1 deletion bedevere/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,11 @@ async def files_for_PR(gh, pull_request):


async def issue_for_PR(gh, pull_request):
"""Get the issue data for a pull request."""
"""Get the issue_url for a pull request.

The issue_url is the API endpoint for the given pull_request.
This terminology follows the official Github API and its documentation.
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this doesn't just get the URL of the API endpoint, but (unless I'm misunderstanding again) it also retrieves the JSON data from that endpoint, returning a dict that contains the info/data for the PR (hence my suggestion about calling it get_pr_info/get_pr_data).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, my bad. You're right. What about:

Suggested change
"""Get the issue_url for a pull request.
The issue_url is the API endpoint for the given pull_request.
This terminology follows the official Github API and its documentation.
"""
"""Get the issue_url for a pull request.
The issue_url is a JSON payload with all data relevant for the given pull_request.
This terminology follows the official Github API and its documentation.
"""

return await gh.getitem(pull_request["issue_url"])
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved


Expand Down
2 changes: 1 addition & 1 deletion tests/test_filepaths.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ async def test_docs_only(author_association):
check_n_pop_nonews_events(gh, author_association == 'NONE')
assert len(gh.post_url) == 1
assert gh.post_url[0] == 'https://api.github.com/some/label'
assert gh.post_data[0] == [Labels.docs.value]
assert gh.post_data[0] == [Labels.docs.value, Labels.skip_news.value]


@pytest.mark.parametrize('author_association', ['OWNER', 'MEMBER', 'CONTRIBUTOR', 'NONE'])
Expand Down
28 changes: 25 additions & 3 deletions tests/test_prtype.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ async def test_docs_no_news():
assert gh.getitem_url == 'https://api.github.com/repos/cpython/python/issue/1234'
assert len(gh.post_url) == 1
assert gh.post_url[0] == 'https://api.github.com/some/label'
assert gh.post_data[0] == [Labels.docs.value]
assert gh.post_data[0] == [Labels.docs.value, Labels.skip_news.value]


async def test_docs_and_news():
Expand Down Expand Up @@ -154,7 +154,7 @@ async def test_docs_and_tests():

async def test_leave_existing_type_labels():
filenames = {'/path/to/docs.rst', 'test_docs2.py'}
issue = {'labels': [{'name': 'skip news'}, {'name': 'type-documentation'}],
issue = {'labels': [{'name': 'skip news'}, {'name': 'docs'}],
'labels_url': 'https://api.github.com/some/label'}
gh = FakeGH(getiter=filenames, getitem=issue)
event_data = {
Expand All @@ -168,7 +168,29 @@ async def test_leave_existing_type_labels():
}
await prtype.classify_by_filepaths(gh, event_data['pull_request'], filenames)
assert gh.getitem_url == 'https://api.github.com/repos/cpython/python/issue/1234'
# Only creates type-tests label.
assert len(gh.post_url) == 1
assert gh.post_url[0] == "https://api.github.com/some/label"
# This should only add the tests label as the docs label is already applied
assert gh.post_data[0] == [Labels.tests.value]


async def test_do_not_post_if_nothing_to_apply():
filenames = {'/path/to/docs.rst'}
issue = {'labels': [{'name': 'skip news'}, {'name': 'docs'}],
'labels_url': 'https://api.github.com/some/label'}
gh = FakeGH(getiter=filenames, getitem=issue)
event_data = {
'action': 'opened',
'number': 1234,
'pull_request': {
'url': 'https://api.github.com/repos/cpython/python/pulls/1234',
'statuses_url': 'https://api.github.com/some/status',
'issue_url': 'https://api.github.com/repos/cpython/python/issue/1234',
},
}
await prtype.classify_by_filepaths(gh, event_data['pull_request'], filenames)
assert gh.getitem_url == 'https://api.github.com/repos/cpython/python/issue/1234'
# This should not post anything as docs is already applied
assert len(gh.post_url) == 0


Expand Down