Skip to content
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

Download release binary #2

Merged
merged 5 commits into from
Mar 13, 2024
Merged

Download release binary #2

merged 5 commits into from
Mar 13, 2024

Conversation

ben-harvey
Copy link
Contributor

This change updates the action code to download the correct release binary from our dedicated release repo at https://github.com/Enterprise-CMCS/mac-fc-report-event-releases. It adds a version input that uses semver to parse the input, and a token input that uses the GITHUB_TOKEN as its default. This lets us get around the low rate limit imposed by the GitHub API on unauthenticated requests.

Testing:

  • ran the action locally and via the test-action workflow
  • confirmed that the GITHUB_TOKEN fixed the rate-limiting issue
  • provided some unsuccessful version constraints to test error handling

@ben-harvey ben-harvey requested a review from hundt-corbalt March 5, 2024 21:53
Copy link
Contributor

@hundt-corbalt hundt-corbalt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool! one little comment

src/index.ts Outdated
const artifact = artifacts.find(
(a) => a.goos === goOs && a.goarch === goArch
const asset = validRelease.assets.find(
(a) => a.name.includes(nodeArch) && a.name.includes(nodeType)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should be able to just construct the expected binary name and check for an exact match right? that seems a little more straightforward and less likely to lead to something weird happening

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call

@ben-harvey ben-harvey force-pushed the bharvey-get-releases branch from e6bd2ad to 640f4a4 Compare March 7, 2024 20:19
@ben-harvey ben-harvey requested a review from hundt-corbalt March 7, 2024 20:21
Copy link
Contributor

@hundt-corbalt hundt-corbalt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ben-harvey ben-harvey merged commit ccec69f into main Mar 13, 2024
4 checks passed
@ben-harvey ben-harvey deleted the bharvey-get-releases branch March 13, 2024 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants