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

fix: remove broken executable "resolver" for arm64 arch #7013

Closed
wants to merge 2 commits into from

Conversation

domoritz
Copy link

Same as #6914 but with a different commit message.

With this change, we don't need to patch puppeteer on M1. See #6622.

Same as puppeteer#6914 but with a different commit message.

With this change, we don't need to patch puppeteer on M1. See puppeteer#6622.
@google-cla google-cla bot added the cla: yes label Mar 22, 2021
@domoritz
Copy link
Author

I don't think the test failure is related to this change.

@domoritz
Copy link
Author

domoritz commented Apr 9, 2021

@jackfranklin can you take a look at this?

@domoritz
Copy link
Author

@theDavidBarton could you take a look?

Copy link
Contributor

@theDavidBarton theDavidBarton left a comment

Choose a reason for hiding this comment

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

@theDavidBarton could you take a look?

I have taken a look gladly but I have no write access on the project, so I let @jackfranklin or other members approve it in a second round.

Note: the very same change was already approved by Mathias back in February 2021, see #6914 (review). This time commit-msg is OK.

@domoritz
Copy link
Author

Thank you @theDavidBarton. I randomly picked you since I didn't know a better way to get the attention of someone who has write access.

I apologize if there is a process for getting reviews that I missed.

@theDavidBarton
Copy link
Contributor

Thank you @theDavidBarton. I randomly picked you since I didn't know a better way to get the attention of someone who has write access.

I apologize if there is a process for getting reviews that I missed.

No worries 😊
I am sure soon it will be merged. Because of the M1 involvement, it is a very useful fix.

@jschfflr
Copy link
Contributor

@domoritz It has been a while since this change has been uploaded. Could you verify that it's still needed?
If yes, I'm happy to get this landed :)

@jschfflr jschfflr self-requested a review September 11, 2021 08:46
@domoritz
Copy link
Author

I don't think it is with #7099.

@domoritz domoritz closed this Sep 12, 2021
@domoritz domoritz deleted the patch-1 branch September 12, 2021 12:48
@jschfflr
Copy link
Contributor

Thanks @domoritz!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants