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

dynamically add healthkit entitlements #83

Merged
merged 5 commits into from
Feb 7, 2017

Conversation

johnborges
Copy link
Contributor

As of [email protected] you can make direct use of a debug and release entitlement plist to dynamically add entitlements. No need for hooks. This plugin can directly add the entitlements it needs. #79

plugin.xml Outdated
@@ -21,6 +21,7 @@

<engines>
<engine name="cordova" version=">=3.0.0"/>
<engine name="cordova-ios" version=">=4.3.0" />
Copy link
Owner

Choose a reason for hiding this comment

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

I ❤️ that we can do this, but can we remove this restriction to not impose 4.3.0+ on people? I guess those config-file lines will just be ignored on lower versions and we can update the readme to state manually adding the entitlements is only required for < 4.3.0.

WDYT?

Thanks,
Eddy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that is a good idea as long as those config-file lines get ignored gracefully. I'll try and test it out cordova-ios < 4.3.0

Copy link
Owner

Choose a reason for hiding this comment

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

Would be awesome if that's the case!

@johnborges
Copy link
Contributor Author

So it looks like on older projects which make use of cordova-ios<4.3.0 the plugin works fine and simply ignores the references made to Entitlements-Debug and Entitlements-Release. No need to enforce [email protected] requirement.

@EddyVerbruggen
Copy link
Owner

That's perfect! I guess we're ready to merge then, right?

@johnborges
Copy link
Contributor Author

yup

@EddyVerbruggen EddyVerbruggen merged commit cb21849 into EddyVerbruggen:master Feb 7, 2017
@EddyVerbruggen
Copy link
Owner

Awesomness, will release to npm soon 🎉

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.

2 participants