A new plugin to post a welcome comment#94
Conversation
Cadair
force-pushed
the
welcome_comment
branch
from
aa8461b to
9ab5956
Compare
Cadair
force-pushed
the
welcome_comment
branch
from
02c8268 to
c3247fc
Compare
Codecov Report
@@ Coverage Diff @@
## master #94 +/- ##
=========================================
- Coverage 82.34% 81.4% -0.95%
=========================================
Files 17 18 +1
Lines 912 925 +13
=========================================
+ Hits 751 753 +2
- Misses 161 172 +11
Continue to review full report at Codecov.
|
| for function, actions in PULL_REQUEST_CHECKS.items(): | ||
| if actions is None or action in actions: | ||
| result = function(pr_handler, repo_handler) | ||
| result = function(pr_handler, repo_handler, payload) |
There was a problem hiding this comment.
Does this need to break backward compat? Can this be an optional keyword as above?
There was a problem hiding this comment.
this isn't a function sig, we are calling the registered callback functions. The only way to pass this information is to always pass it. We could do some crazy signature introspection stuff but...
|
|
||
| def process_pull_request(repository, number, installation, action, | ||
| is_new=False): | ||
| is_new=False, *, payload): |
There was a problem hiding this comment.
Does this mean payload has to be a keyword? If so, why is no default assigned?
There was a problem hiding this comment.
because I didn't want to change the signature, but we always should pass payload in.
There was a problem hiding this comment.
The other change is already a breaking change, so why not just change the signature here also?
There was a problem hiding this comment.
I personally think it makes sense like this though, as a required keyword argument
|
I think there is actually a better way to achieve this, without the breaking change. However, I do wonder if passing the full webhook payload through is still a good idea. |
|
Maaaaaybe this should be GitHub Action... |
4 participants
This just posts a comment on all PRs only on open events. It might be nice to be able to limit this to new contributors, or exclude people who are in teams in the org or something. However, for now this should suffice.
Based on #87 and needs docs.