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

sha256: handle different values for x64 and arm64 #10

Merged

Conversation

ldennington
Copy link
Collaborator

@ldennington ldennington commented Jul 20, 2022

Allow for multiple different sha256 values for x64 and arm64 assets.
Additionally, clean up certain items (unneeded using statements, unused
method parameters, etc.) that cause linting to fail.

I tested these changes with this workflow which produced this PR.

@mjcheetham
Copy link
Owner

In Cask Ruby file that we're targeting we have this section still only using the Intel Homebrew prefix:

  uninstall script: {
                      executable: '/usr/local/share/gcm-core/uninstall.sh',
                      sudo:       true,
                    },
            pkgutil: 'com.microsoft.GitCredentialManager'

https://github.com/ldennington/homebrew-tap/blob/123487f73f5c3a77681bc2701542c9a7880f687f/Casks/git-credential-manager-core.rb#L17-L21

@ldennington
Copy link
Collaborator Author

In Cask Ruby file that we're targeting we have this section still only using the Intel Homebrew prefix:

  uninstall script: {
                      executable: '/usr/local/share/gcm-core/uninstall.sh',
                      sudo:       true,
                    },
            pkgutil: 'com.microsoft.GitCredentialManager'

https://github.com/ldennington/homebrew-tap/blob/123487f73f5c3a77681bc2701542c9a7880f687f/Casks/git-credential-manager-core.rb#L17-L21

D'oh. Should be a pretty easy fix at least. Since it's static I'll just go ahead and change it over in microsoft/homebrew-git.

Copy link
Owner

@mjcheetham mjcheetham left a comment

Choose a reason for hiding this comment

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

Looks like this works! I did have one suggestion about how we might be able to avoid baking-in assumptions about the asset names (containing the RID) that may work.
Let me know what you think!

src/main.ts Outdated Show resolved Hide resolved
src/main.ts Outdated Show resolved Hide resolved
src/main.ts Outdated Show resolved Hide resolved
src/main.ts Outdated Show resolved Hide resolved
src/homebrew.ts Outdated Show resolved Hide resolved
src/main.ts Outdated Show resolved Hide resolved
@ldennington ldennington force-pushed the update-release-asset-regex branch 2 times, most recently from e94c2f9 to 1b9c126 Compare July 28, 2022 19:29
@ldennington
Copy link
Collaborator Author

@mjcheetham - I think we're good to go here 🤞🏻. Here's my latest test run with these changes: https://github.com/ldennington/homebrew-tap/pull/23/files.

@ldennington
Copy link
Collaborator Author

@mjcheetham - would you mind approving the workflows for this PR? Also, if you have time to take a look at my updates since the last review I'd appreciate it!

@mjcheetham
Copy link
Owner

@mjcheetham - would you mind approving the workflows for this PR? Also, if you have time to take a look at my updates since the last review I'd appreciate it!

Approved, and also invited you to be a collaborator so you have more permissions on this repo for the future :-)

Copy link
Owner

@mjcheetham mjcheetham left a comment

Choose a reason for hiding this comment

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

This looks good! The build error is:

src/main.ts(40,31): error TS2339: Property 'getMultilineInput' does not exist on type 'typeof import("/home/runner/work/update-homebrew/update-homebrew/node_modules/@actions/core/lib/core")'.

Might need to update the dependencies/lock file or something? 🤷

@ldennington ldennington force-pushed the update-release-asset-regex branch 2 times, most recently from 453d026 to a0abda8 Compare August 4, 2022 19:57
Allow for multiple different sha256 values for x64 and arm64 assets. This
means allowing specification of multiple regexes in the releaseAsset
input. Additionally, clean up certain items (unneeded using statements,
unused method parameters, etc.) that cause linting to fail.
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.

None yet

2 participants