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

✨ Install the correct Chromium binary for arm Macs #486

Merged
merged 3 commits into from
Aug 10, 2021

Conversation

wwilsman
Copy link
Contributor

What is this?

For a while, Chromium for Mac was only built on x64 architecture. With the advent of new M1 arm Macs, it wasn't clear if the Percy CLI even worked on these new macs since we rely on Chromium for asset discovery. I looked back into the Google storage APIs and found that there were recent additions that added Mac arm support. This PR adds the appropriate changes so our install script will download the correct version of Chromium when run on arm based Macs.

@wwilsman wwilsman added the ✨ enhancement New feature or request label Aug 10, 2021
@wwilsman wwilsman requested a review from Robdel12 August 10, 2021 17:24

for (let rev = state.range[1]; rev >= state.range[0]; rev--) {
for (; rev >= state.range[0]; rev--) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got an error with this variable using the latest LTS node with this script. Apparently, let hoisting has changed under the hood. The change makes sense though, as we're referencing rev outside of the loop down below.

@wwilsman wwilsman enabled auto-merge (squash) August 10, 2021 17:27
@wwilsman wwilsman disabled auto-merge August 10, 2021 17:28
@wwilsman wwilsman enabled auto-merge (squash) August 10, 2021 17:28
Copy link
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

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

🏁

@wwilsman wwilsman disabled auto-merge August 10, 2021 17:37
@wwilsman wwilsman enabled auto-merge (squash) August 10, 2021 17:41
@wwilsman wwilsman merged commit 882b4ae into master Aug 10, 2021
@wwilsman wwilsman deleted the ww/install-chromium-mac-arm branch August 10, 2021 18:27
samarsault pushed a commit that referenced this pull request Mar 3, 2023
Bumps [eslint-plugin-import](https://github.com/import-js/eslint-plugin-import) from 2.25.4 to 2.26.0.
- [Release notes](https://github.com/import-js/eslint-plugin-import/releases)
- [Changelog](https://github.com/import-js/eslint-plugin-import/blob/main/CHANGELOG.md)
- [Commits](import-js/eslint-plugin-import@v2.25.4...v2.26.0)

---
updated-dependencies:
- dependency-name: eslint-plugin-import
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants