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

bpo-to-github-migration replace roundup summary script #91738

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Harry-Lees
Copy link
Contributor

References: psf/gh-migration#6

Currently, this script requires the following template in MailGun:

<h1>ACTIVITY SUMMARY</h1>
<p>({{ timespan }})</p>
<p>Python issue tracker at <a href="https://github.com/python/cpython/issues">https://github.com/python/cpython/issues</a></p>

<p>To view or respond to any of the issues listed below, click on the issue. Do NOT respond to this message.</p>

<br>

<p>Issues stats:</p>
<table border="1">
    <tr>
        <th>open</th>
        <td>{{ total_open }} ({{len opened_issues }})</td>
    </tr>
    <tr>
        <th>closed</th>
        <td>{{ total_closed }} ({{len closed_issues }})</td>
    </tr>
    <tr>
        <th>total</th>
        <td>{{sum total_open total_closed }} ({{ total_diff }})</td>
    </tr>
</table>

<br>

<p>Issues Opened ({{len opened_issues }})</p>
=======================================
{{#each opened_issues}}
<p>#{{ this.number }}: {{ this.title }}<br>
<a href="https://github.com/python/cpython/issues/{{ this.number }}">https://github.com/python/cpython/issues/{{ this.number }}</a> opened by {{ this.author.login }}</p>
{{/each}}

<br>

<p>Top 10 most discussed issues ({{len most_discussed }})</p>
=======================================
{{#each most_discussed }}
<p>#{{ this.number }}: {{ this.title }}<br>
<a href="https://github.com/python/cpython/issues/{{ this.number }}">https://github.com/python/cpython/issues/{{ this.number }}</a> opened by {{ this.author.login }}</p>
{{/each}}

<br>

<p>Most recent 15 issues with no replies ({{len no_comments }})</p>
=======================================
{{#each no_comments }}
<p>#{{ this.number }}: {{ this.title }}<br>
<a href="https://github.com/python/cpython/issues/{{ this.number }}">https://github.com/python/cpython/issues/{{ this.number }}</a> opened by {{ this.author.login }}</p>
{{/each}}

<br>

<p>Issues Closed ({{len closed_issues }})</p>
=======================================
{{#each closed_issues}}
<p>#{{ this.number }}: {{ this.title }}<br>
<a href="https://github.com/python/cpython/issues/{{ this.number }}">https://github.com/python/cpython/issues/{{ this.number }}</a> opened by {{ this.author.login }}</p>
{{/each}}

Furthermore, the following helpers were used to calculate number of issues etc.

Handlebars.registerHelper('sum', function (a, b) {
    return a + b
})

Handlebars.registerHelper('len', function(arr) {
    return arr.length
})

@Harry-Lees
Copy link
Contributor Author

I've left the "to" field blank in the params in send_report(), I assume this will be set to [email protected]?

@Harry-Lees
Copy link
Contributor Author

Is there anything else I need to do to get this merged?

@ezio-melotti
Copy link
Member

Is there anything else I need to do to get this merged?

I think we need @ewdurbin to set up the template and double-check the code.

FTR another alternative was proposed in this post: https://discuss.python.org/t/triaging-reviewing-fixing-issues-and-prs/15365/18
I think we should still merge this PR, and either expand and improve on it if it works well for our use case, or possibly end up replacing it with the one from the post if it is better (also from a maintenance point of view).


def get_issues(filters: Iterable[str], token: str, all_: bool = True):
"""return a list of results from the Github search API"""
# TODO: if there are more than 100 issues, we need to include pagination
Copy link
Member

Choose a reason for hiding this comment

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

Should this TODO be addressed before merge? If not is there something we should put in place to make clear that there weren't magically just 100 issues a given week?

Copy link
Member

@ewdurbin ewdurbin left a comment

Choose a reason for hiding this comment

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

Thanks for the work to implement this report @Harry-Lees!

I have some concerns that are raised in review, but one of the primary ones is that there is no clear way to add the helper functions you mention in the PR description that I can find in the Mailgun UI. Are they required, or can you work around without them?

"""send the report using the Mailgun API"""
params = {
"from": "Cpython Issues <[email protected]>",
"to": "",
Copy link
Member

Choose a reason for hiding this comment

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

Should be configured before merge.

"from": "Cpython Issues <[email protected]>",
"to": "",
"subject": "Summary of Python tracker Issues",
"template": "issue-tracker-template",
Copy link
Member

Choose a reason for hiding this comment

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

Note To Self: Ensure template name matches when added to Mailgun.

github_token,
False)

payload = {
Copy link
Member

Choose a reason for hiding this comment

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

This payload appears to be missing the following keys to render the proposed template:

  • timespan
  • total_diff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants