-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
refactor into modules #53
Open
wgwz
wants to merge
2
commits into
master
Choose a base branch
from
wgwz/events
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
"""Github Event handlers for the snekomatic app.""" | ||
import gidgethub | ||
from glom import glom | ||
from .gh import GithubApp | ||
from .db import PersistentStringSet | ||
from .messaging import invite_message | ||
|
||
|
||
github_app = GithubApp() | ||
SENT_INVITATION = PersistentStringSet("SENT_INVITATION") | ||
|
||
|
||
async def _member_state(gh_client, org, member): | ||
# Returns "active" (they're a member), "pending" (they're not a member, | ||
# but they have an invitation they haven't responded to yet), or None | ||
# (they're not a member and don't have a pending invitation) | ||
try: | ||
response = await gh_client.getitem( | ||
"/orgs/{org}/memberships/{username}", | ||
url_vars={"org": org, "username": member}, | ||
) | ||
except gidgethub.BadRequest as exc: | ||
if exc.status_code == 404: | ||
return None | ||
else: | ||
raise | ||
else: | ||
return glom(response, "state") | ||
|
||
|
||
# There's no "merged" event; instead you get action=closed + merged=True | ||
@github_app.route("pull_request", action="closed") | ||
async def pull_request_merged(event_type, payload, gh_client): | ||
print("PR closed") | ||
if not glom(payload, "pull_request.merged"): | ||
print("but not merged, so never mind") | ||
return | ||
creator = glom(payload, "pull_request.user.login") | ||
org = glom(payload, "organization.login") | ||
print(f"PR by {creator} was merged!") | ||
|
||
if creator in SENT_INVITATION: | ||
print("The database says we already sent an invitation") | ||
return | ||
|
||
state = await _member_state(gh_client, org, creator) | ||
if state is not None: | ||
# Remember for later so we don't keep checking the Github API over and | ||
# over. | ||
SENT_INVITATION.add(creator) | ||
print(f"They already have member state {state}; not inviting") | ||
return | ||
|
||
print("Inviting! Woohoo!") | ||
# Send an invitation | ||
await gh_client.put( | ||
"/orgs/{org}/memberships/{username}", | ||
url_vars={"org": org, "username": creator}, | ||
data={"role": "member"}, | ||
) | ||
# Record that we did | ||
SENT_INVITATION.add(creator) | ||
# Welcome them | ||
await gh_client.post( | ||
glom(payload, "pull_request.comments_url"), | ||
data={"body": invite_message.format(username=creator)}, | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
"""Messaging for the snekomatic app.""" | ||
# dedent, remove single newlines (but not double-newlines), remove | ||
# leading/trailing newlines | ||
|
||
|
||
def _fix_markdown(s): | ||
import textwrap | ||
|
||
s = s.strip("\n") | ||
s = textwrap.dedent(s) | ||
s = s.replace("\n\n", "__PARAGRAPH_BREAK__") | ||
s = s.replace("\n", " ") | ||
s = s.replace("__PARAGRAPH_BREAK__", "\n\n") | ||
return s | ||
|
||
|
||
invite_message = _fix_markdown( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could as well just have a Jinja template instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is a good point! I would like to add that in as well. |
||
""" | ||
Hey @{username}, it looks like that was the first time we merged one of | ||
your PRs! Thanks so much! :tada: :birthday: | ||
|
||
If you want to keep contributing, we'd love to have you. So, I just sent | ||
you an invitation to join the python-trio organization on Github! If you | ||
accept, then here's what will happen: | ||
|
||
* Github will automatically subscribe you to notifications on all our | ||
repositories. (But you can unsubscribe again if you don't want the | ||
spam.) | ||
|
||
* You'll be able to help us manage issues (add labels, close them, etc.) | ||
|
||
* You'll be able to review and merge other people's pull requests | ||
|
||
* You'll get a [member] badge next to your name when participating in the | ||
Trio repos, and you'll have the option of adding your name to our | ||
[member's page](https://github.com/orgs/python-trio/people) and putting | ||
our icon on your Github profile | ||
([details](https://help.github.com/en/articles/publicizing-or-hiding-organization-membership)) | ||
|
||
If you want to read more, [here's the relevant section in our contributing | ||
guide](https://trio.readthedocs.io/en/latest/contributing.html#joining-the-team). | ||
|
||
Alternatively, you're free to decline or ignore the invitation. You'll | ||
still be able to contribute as much or as little as you like, and I won't | ||
hassle you about joining again. But if you ever change your mind, just let | ||
us know and we'll send another invitation. We'd love to have you, but more | ||
importantly we want you to do whatever's best for you. | ||
|
||
If you have any questions, well... I am just a [humble Python | ||
script](https://github.com/python-trio/snekomatic), so I probably can't | ||
help. But please do post a comment here, or [in our | ||
chat](https://gitter.im/python-trio/general), or [on our | ||
forum](https://trio.discourse.group/c/help-and-advice), whatever's | ||
easiest, and someone will help you out! | ||
|
||
""" | ||
) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not "events", it's
conceptionally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose we might want the name to include "github" too, since we might have other kinds of events later?
I also guess we might eventually want to refactor along the lines of logically-related-features, like "inviting new users", "dependency bumping", etc., instead of lower-level functional categories like "reacting to github events", "text strings", etc.
But no need to make the perfect the enemy of the good... I'm happy with merging this now and then worrying about that stuff later. @wgwz, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like both of the suggestions! I definitely prefer taking it one step further and splitting up the modules into logically related features. I'm willing to do that for this MR.