-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Add ListWorkflowJobsAttempt method to ActionsService #3060
Add ListWorkflowJobsAttempt method to ActionsService #3060
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3060 +/- ##
=======================================
Coverage 97.71% 97.71%
=======================================
Files 152 152
Lines 13326 13342 +16
=======================================
+ Hits 13021 13037 +16
Misses 215 215
Partials 90 90 ☔ 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.
Thank you, @fchimpan !
This looks like a great start apart from the code coverage.
I've added a suggestion that should hopefully clear up the coverage tests.
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.
Thank you, @fchimpan !
LGTM
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
Thanks for your advice. I was already working on improving the test coverage before I noticed your comments, so there may have been some overlap. I would appreciate it if you could review the changes. I look forward to your feedback. Best regards, |
@fchimpan - while you are here, would you mind taking a look at other open PRs in this repo with the "Needs Review" label and reviewing them? I know that one person was waiting for #3049 in particular, but all of them need review at some point... feel free to pick and choose (or leave them alone - your choice). |
@gmlewis Since I don't have the authority to approve pull requests, should I just leave comments on the PR if I have any suggestions? |
Thank you, @fchimpan ! |
Thank you @gmlewis By the way, sometimes I come across APIs that are not yet implemented. In such cases, is it alright if I propose a PR like I did this time? Thank you in advance. |
Absolutely! We welcome all contributions. |
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!
Thank you, @valbeat ! |
Fixes: #3059.
Add support ListWorkflowJobsAttempt API.
https://docs.github.com/rest/actions/workflow-jobs#list-jobs-for-a-workflow-run-attempt