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

test build with updated github for apple silicon #124

Closed

Conversation

benonymus
Copy link
Contributor

I want to test out the updated github library on a real build, to verify the fix.

@benonymus benonymus force-pushed the updated-github-test-build branch from 0ab4c7e to 7796736 Compare November 11, 2022 03:08
@benonymus benonymus force-pushed the updated-github-test-build branch from 05ccabc to 29140dc Compare November 12, 2022 12:58
@benonymus benonymus force-pushed the updated-github-test-build branch from 29140dc to c6fce33 Compare November 12, 2022 13:59
}
}
extraMetadata: {},
asarUnpack: ["node_modules/github/bin/*"],
Copy link
Contributor

Choose a reason for hiding this comment

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

The extraMetadata is overwritten in the main function so this has no effect

Copy link
Contributor Author

@benonymus benonymus Nov 12, 2022

Choose a reason for hiding this comment

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

extraMetadata was here before, we could remove it then.
I only added asarUnpack.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤦 I thought asarUnpack was inside extraMetadata. Sorry for that.

Copy link
Contributor

Choose a reason for hiding this comment

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

With the GitHub fixes, do we actually need this asarUnpack key?

@benonymus
Copy link
Contributor Author

benonymus commented Nov 12, 2022 via email

@jonian
Copy link
Contributor

jonian commented Nov 12, 2022

@benonymus, you should open a new PR for asarUnpack since it resolves another issue not related to apple silicon.

@benonymus
Copy link
Contributor Author

benonymus commented Nov 12, 2022

@benonymus, you should open a new PR for asarUnpack since it resolves another issue not related to apple silicon.

Yeah that is fair. If we say this this is fine by others as well.

This additional issue was only discovered after I made the fix for Apple Silicon.

It fixes this: https://discord.com/channels/992103415163396136/992103415163396139/1040564640062648370

Here is the pr: #140

@benonymus
Copy link
Contributor Author

benonymus commented Nov 12, 2022

Technically this pr can be closed. I got what I wanted to try.
I think we should merge the pr on the GitHub repo and have a new tag. Then add that to Pulsar instead of my fork.

pulsar-edit/github#2

What do you think @mauricioszabo ?

@benonymus benonymus mentioned this pull request Dec 11, 2022
@confused-Techie
Copy link
Member

Like stated above that this should be closed, now especially as we have #186 merged, should we close this one @benonymus?

@benonymus benonymus closed this Dec 11, 2022
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.

5 participants