-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
feat: add support for Mojave app notarization #899
Conversation
d5ed264
to
f5f9d67
Compare
Codecov Report
@@ Coverage Diff @@
## master #899 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 12 12
Lines 641 671 +30
=====================================
+ Hits 641 671 +30
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if we can't test the actual use of electron-notarize
, we should be able to test things like:
- it doesn't work when notarize is true but sign is not
- users can't pass nested fields like
appBundleId
andappPath
toelectron-notarize
@MarshallOfSound this PR is dependent upon #900 due to the version of Node |
f5f9d67
to
58c1453
Compare
What are the minimum macOS/Xcode versions? Travis added Xcode 10.1 recently. |
@MarshallOfSound I added tests (and moved some code around in the process). Could you take a look to see that the tests are the expected behavior? |
@malept Seems legit to me, thanks for picking that up 👍 |
[] The changes have sufficient test coverage (if applicable).Summarize your changes:
Functionality comes from
electron-notarize
We can't really add tests for this because it requires a legit apple ID and no CI system is updated to a high enough macOS / Xcode version to run tests 😆