-
Notifications
You must be signed in to change notification settings - Fork 4
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 pr reminder v2 #82
Conversation
Temporary new file name
Added tests for `schedule_jobs` and `slash_prs` in `events.py`
Also, removed duplicate function
Added test case for when an unknown user calls the command
These errors have no usages so we can remove them.
Filter method should be a separate call to run.
Using channel value from thread message response data
Minor
No need to reuse this code as it only occurs twice
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #82 +/- ##
==========================================
+ Coverage 94.97% 96.52% +1.55%
==========================================
Files 19 13 -6
Lines 776 634 -142
==========================================
- Hits 737 612 -125
+ Misses 39 22 -17 ☔ View full report in Codecov by Sentry. |
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.
LGTM - had a chat with @khalford for how to refactor this code for extensibility so that would be the next step as we extend this to look at gitlab or send messages to places other than slack
Description:
Rewrite of PR reminder feature:
Merged
pr_reminder.py
,post_to_dms.py
andbase_feature.py
into one module. The base feature class is not needed anymore as future features will not be inheriting any methods from it, therefore it makes sense to remove it.Submitter:
Have you done the following?:
major | minor | patch
ordocumentation | workflow
dev-cloud-chatops.nubes.rl.ac.uk
?New / Existing features:
version.txt
anddocker-compose.yml
files?Reviewer:
Have you checked the following?: