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

Add option for child entitlements #278

Merged
merged 1 commit into from
Mar 8, 2016

Conversation

mcfedr
Copy link
Contributor

@mcfedr mcfedr commented Feb 29, 2016

Allows the entitlements-inherit option of electron-osx-sign to be passed.

Related to #271

@malept malept added enhancement Feature request build-target:mac 🍎 Bundling an Electron app specifically for macOS docs-needed 📝 Pull request needs documentation labels Feb 29, 2016
@malept
Copy link
Member

malept commented Feb 29, 2016

Thanks for the contribution! Could you please add an entry to usage.txt?

@mcfedr
Copy link
Contributor Author

mcfedr commented Feb 29, 2016

I didn't re-indent the whole file right away, it pushes everything very far to the right, what do you think @malept ?

@malept
Copy link
Member

malept commented Feb 29, 2016

Hmm. Try moving the description to the next line.

Also, un-capitalize "The", it's inconsistent with the rest of the argument descriptions.

@mcfedr mcfedr force-pushed the child-entitlements branch from 028d945 to 4e92e03 Compare February 29, 2016 15:48
@mcfedr
Copy link
Contributor Author

mcfedr commented Feb 29, 2016

ah, I even looked at the capitals and thought it should be small.


`sign-entitlements-inherit` - *String*

The path to the 'child' entitlements. See [electron-osx-sign](https://www.npmjs.com/package/electron-osx-sign) for more details.
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Also, you should add the same caveat as sign-entitlements:

(Currently limited to Mac App Store distribution.)

Copy link
Contributor

Choose a reason for hiding this comment

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

@malept The signing with entitlements should now work fine with darwin builds in the latest electron-osx-sign v0.3.0.

Copy link
Member

Choose a reason for hiding this comment

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

@sethlu thanks for the correction. I guess the caveat needs to be removed from the entitlements docs, then. That can happen outside of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

@malept That may be better I think.
I'll find a way to explain the entitlements for darwin builds as they, apart from sandboxing an app, do not actually do much except interacting with iCloud services, iAP or things like that. It's pretty hard to access some of these Apple services with current builds of Electron, so basically entitlements could be ignored for darwin. The default entitlements for darwin is blank: If a darwin build is sandboxed, it should not also really function properly in the presence of frameworks like Squirrel.framework.

@malept malept modified the milestone: 6.0.0 Mar 5, 2016
@malept malept removed the docs-needed 📝 Pull request needs documentation label Mar 8, 2016
@malept
Copy link
Member

malept commented Mar 8, 2016

Looks good. Squash the commits and I'll merge.

@mcfedr mcfedr force-pushed the child-entitlements branch from 68336ed to 623b068 Compare March 8, 2016 11:45
@mcfedr
Copy link
Contributor Author

mcfedr commented Mar 8, 2016

@malept squashed

@malept
Copy link
Member

malept commented Mar 8, 2016

Thanks!

malept added a commit that referenced this pull request Mar 8, 2016
Add option for child entitlements
@malept malept merged commit 65a5cdb into electron:master Mar 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-target:mac 🍎 Bundling an Electron app specifically for macOS enhancement Feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants