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

Github comments when user is warned/removed (in addition to the emails that are sent when this happens) #897

Merged
merged 5 commits into from
Apr 16, 2018

Conversation

owocki
Copy link
Contributor

@owocki owocki commented Apr 16, 2018

Description

Github comments from gitcoinbot when a user is removed from the issue.

example issue

This gives context to anyone looking back at the thread to try and figure out what is going on with the issue that the github bot has kicked them off of the issue.

In addition, this PR will enable notifications to slack so that we can deal with it internally

Checklist
  • linter status: 100% pass
  • changes don't break existing behavior
  • commit message follows commit guidelines
Affected core subsystem(s)

bot

Testing

tested

Refers/Fixes

self

@owocki
Copy link
Contributor Author

owocki commented Apr 16, 2018

another route we could go is just to disable the auto-kick functinoality. instead, we could make the bot just warn the user in perpetuity

@owocki owocki changed the title Github comments when user is removed Github comments when user is warned/removed (in addition to the emails that are sent when this happens) Apr 16, 2018
@codecov
Copy link

codecov bot commented Apr 16, 2018

Codecov Report

Merging #897 into master will decrease coverage by 14.69%.
The diff coverage is 12.24%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #897      +/-   ##
==========================================
- Coverage   33.52%   18.83%   -14.7%     
==========================================
  Files          99       98       -1     
  Lines        6073     6463     +390     
  Branches      731      815      +84     
==========================================
- Hits         2036     1217     -819     
- Misses       3946     5235    +1289     
+ Partials       91       11      -80
Impacted Files Coverage Δ
...eting/management/commands/expiration_start_work.py 15.87% <33.33%> (-63.44%) ⬇️
app/dashboard/notifications.py 8.28% <9.3%> (-6.17%) ⬇️
app/external_bounties/router.py 0% <0%> (-100%) ⬇️
app/external_bounties/forms.py 0% <0%> (-100%) ⬇️
app/app/urls.py 0% <0%> (-93.55%) ⬇️
app/app/sitemaps.py 0% <0%> (-72.5%) ⬇️
app/dashboard/tokens.py 33.33% <0%> (-66.67%) ⬇️
app/marketing/management/commands/sync_keywords.py 29.16% <0%> (-66.67%) ⬇️
app/marketing/management/commands/expiration.py 33.33% <0%> (-57.15%) ⬇️
app/marketing/management/commands/roundup.py 29.41% <0%> (-52.95%) ⬇️
... and 32 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 924aea4...359e90a. Read the comment docs.

@owocki owocki requested a review from mbeacom April 16, 2018 14:23
from github.utils import get_interested_actions
from marketing.mails import bounty_startwork_expire_warning, bounty_startwork_expired
from marketing.mails import (
bounty_startwork_expire_warning, bounty_startwork_expired, maybe_notify_bounty_user_removed_to_slack,
Copy link
Contributor

Choose a reason for hiding this comment

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

This import will fail. maybe_notify_bounty_user_removed_to_slack and maybe_notify_bounty_user_warned_removed_to_slack

Copy link
Contributor

@mbeacom mbeacom left a comment

Choose a reason for hiding this comment

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

lgtm :shipit:

@owocki owocki merged commit fb15562 into master Apr 16, 2018
@mbeacom mbeacom deleted the kevin/removeuser_github_comment branch April 16, 2018 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants