-
Notifications
You must be signed in to change notification settings - Fork 50
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
JS -> TS #33
JS -> TS #33
Conversation
anarast
commented
Aug 1, 2020
- converts all files from javascript to typescript, adds typescript eslint
- may have broken the prettier lint step
@@ -107,7 +114,7 @@ class AutoUpdater { | |||
return true; | |||
} | |||
|
|||
async prNeedsUpdate(pull) { | |||
async prNeedsUpdate(pull: any): Promise<boolean> { |
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 thought that this type should have been PullsListResponseItem
like for update()
, but using it gives the error Property 'merged' does not exist on type 'PullsListResponseItem'
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.
Maybe a type out of https://github.com/octokit/webhooks is what you're looking for? This isn't sent via an API request, GH actions calls autoupdate with an pull event
Codecov Report
@@ Coverage Diff @@
## master #33 +/- ##
==========================================
+ Coverage 98.62% 98.68% +0.05%
==========================================
Files 3 3
Lines 364 379 +15
Branches 81 84 +3
==========================================
+ Hits 359 374 +15
Misses 5 5
Continue to review full report at Codecov.
|
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.
Looks great! Pretty excited to get this in. Some comments below
@@ -107,7 +114,7 @@ class AutoUpdater { | |||
return true; | |||
} | |||
|
|||
async prNeedsUpdate(pull) { | |||
async prNeedsUpdate(pull: any): Promise<boolean> { |
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.
Maybe a type out of https://github.com/octokit/webhooks is what you're looking for? This isn't sent via an API request, GH actions calls autoupdate with an pull event
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.
One last thing and this is good to be merged!
exec'd into the container and the index.js looked like it had all the stuff |
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.
Nice, LGTM 👍