chore(pr-comments): generalization for triggering merged PR comments workflow #71039
Conversation
github-actions
bot
added
the
Scope: Backend
cathteng
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #71039 +/- ##
==========================================
+ Coverage 78.13% 78.16% +0.03%
==========================================
Files 6670 6672 +2
Lines 298465 298750 +285
Branches 51354 51427 +73
==========================================
+ Hits 233212 233530 +318
+ Misses 58993 58955 -38
- Partials 6260 6265 +5
|
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
cathteng
force-pushed
the
cathy/merged-pr-comments/abstract-trigger-flow
branch
from
66f41ad
to
748aef2
Compare
| @@ -48,6 +48,8 @@ | |||
| PR_COMMENT_TASK_TTL = timedelta(minutes=5).total_seconds() | |||
| PR_COMMENT_WINDOW = 14 # days | |||
|
|
|||
| PR_COMMENT_SUPPORTED_PROVIDERS = {"integrations:github"} | |||
There was a problem hiding this comment.
when all SCMs have implemented the get_merge_commit_sha_from_commit function AND we have a generalized merged PR comment workflow, we can remove this
| response = self.get_pullrequest_from_commit(repo, sha) | ||
| if not response or (isinstance(response, list) and len(response) != 1): | ||
| # the response should return a single merged PR, return if multiple | ||
| return None |
There was a problem hiding this comment.
What does it mean if we return on multiple? Should we be logging it?
There was a problem hiding this comment.
for GitHub, this means there are multiple open PRs with the commit https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#list-pull-requests-associated-with-a-commit
| # the response should return a single merged PR, return if multiple | ||
| return None | ||
|
|
||
| pull_request = response[0] |
There was a problem hiding this comment.
Nit: I like the idiom (pull_request,) = response because it communicates the expectation that the length is 1 and raises otherwise.
If we want to be able to have merged PR comments for SCMs other than GitHub, we'll need to generalize some of the flows. In this PR I generalize the flow to trigger the merged PR comments workflow.
In a follow up PR, I will generalize the workflow itself.