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 now-unusable "legacy" notarization #187

Merged
merged 3 commits into from
May 14, 2024

Conversation

rotu
Copy link
Contributor

@rotu rotu commented Mar 30, 2024

Updated API to reflect deprecated exports.
Updated documentation to match.
Removed code that will no longer work.

@rotu rotu requested a review from a team as a code owner March 30, 2024 20:14
@rotu
Copy link
Contributor Author

rotu commented Mar 30, 2024

note: I took pains to make this backwards-compatible. If you’d rather I remove the obsolete types outright, I could do that instead.

@dsanders11 dsanders11 changed the title Remove now-unusable "legacy" notarization fix: remove now-unusable "legacy" notarization Apr 18, 2024
README.md Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@Kilian
Copy link
Member

Kilian commented Apr 19, 2024

Really appreciate this @rotu!

Had some comments about the docs, someone else has to look at the code bits. I'll leave it to that person whether the error message about legacy should also point to notarytool instead.

@rotu rotu requested a review from Kilian April 25, 2024 04:39
@rotu
Copy link
Contributor Author

rotu commented May 7, 2024

@MarshallOfSound How do I get code review on this?

Copy link
Member

@MarshallOfSound MarshallOfSound left a comment

Choose a reason for hiding this comment

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

Seems legit, @rotu we talked about this and based on our policy RE typescript types you can also remove the types in a follow up if you want. This thing doesn't work, removing it can't be considered semver/major in any form. If anything it just fails peoples builds earlier which IMO is better.

@MarshallOfSound MarshallOfSound merged commit f48a181 into electron:main May 14, 2024
3 checks passed
Copy link

🎉 This PR is included in version 2.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

4 participants