-
Notifications
You must be signed in to change notification settings - Fork 11
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
fix: don't fail when pull_request context in unavailable #207
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #207 +/- ##
==========================================
- Coverage 88.38% 86.69% -1.70%
==========================================
Files 10 10
Lines 689 714 +25
Branches 73 83 +10
==========================================
+ Hits 609 619 +10
- Misses 78 93 +15
Partials 2 2 ☔ 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.
I guess, the conditaion pr.isInitilized() should be added also here:
testing-farm-as-github-action/src/action.ts
Line 250 in 8f6a706
await pr.setStatus( |
@@ -16,7 +16,15 @@ let pr: PullRequest | undefined = undefined; | |||
try { | |||
const octokit = getOctokit(getInput('github_token', { required: true })); | |||
|
|||
pr = await PullRequest.initialize(context.issue.number, octokit); | |||
if (!context.issue || !context.issue.number) { |
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 don't follow this condition? Why we create a new Pull Request if it does not exist
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.
We create an "empty"/"uninitialized" Pull Request and pass it to the action
and post
function.
Pull Request object now provides a new method, isInitialized,
, which returns a boolean and is used to determine if PR data is available. I think that this is cleaner then having everywhere if else conditions. Now it is in one place (PullRequest.isInitialized(): boolean
)
When running on non pull_request like target (e.g. `repository_dispatch`) don't fail and provide some meaningful output. NOTE: Statuses and issue comments are not supported when using such triggers
I think that this PR should make Action more robust even if it doesn't fix the following issue: |
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. I did not hit any issue.
When running on non pull_request a target (e.g.,
repository_dispatch
), don't fail and provide some meaningful output.NOTE: Statuses and issue comments are not supported when using such triggers