-
Notifications
You must be signed in to change notification settings - Fork 510
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 support for specifying latest
for version
#45
Conversation
561aca4
to
e72495c
Compare
e72495c
to
7dbfdef
Compare
2ba2532
to
67582b3
Compare
setup-gcloud/src/version-util.ts
Outdated
@@ -0,0 +1,52 @@ | |||
/* | |||
* Copyright 2019 Google LLC |
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.
nit: year
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.
You caught me copying and pasting :)
Done.
@@ -0,0 +1,30 @@ | |||
/* | |||
* Copyright 2019 Google LLC |
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.
nit: year
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.
Done.
setup-gcloud/src/version-util.ts
Outdated
*/ | ||
export async function getLatestGcloudSDKVersion(): Promise<string> { | ||
const client: httpm.HttpClient = new httpm.HttpClient( | ||
'github-actions-setup-gcloud', |
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.
Can we pass this in from the other fie where it is defined?
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.
Done.
setup-gcloud/src/version-util.ts
Outdated
.get( | ||
'https://api.github.com/repos/GoogleCloudPlatform/cloud-sdk-docker/tags', | ||
) | ||
.then(res => { |
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.
Can we change this to async/await pattern to stay consistent?
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 would love to, sadly the library we're using here doesn't support it, so we have to wrap around it :)
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'm seeing async/await in their "docs" aka tests (https://github.com/microsoft/typed-rest-client/blob/master/test/tests/httptests.ts#L52). This code is just getting harder and harder to read.
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.
Oh snap! You are correct. For some reason when I first looked into using the library I had decided it didn't support it. Glad to see that's not the case. Good catch!
e6b55fa
to
c84e09e
Compare
c84e09e
to
6102f1d
Compare
Integration tests successfully passed: https://github.com/GoogleCloudPlatform/github-actions/actions/runs/32798521
Addresses: #31