You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Anis Kadri <an...@apache.org> on 2013/11/21 04:26:36 UTC
Review Request 15750: Adding/Removing custom frameworks to iOS projects
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15750/
-----------------------------------------------------------
Review request for cordova, Max Woghiren and Braden Shepherdson.
Bugs: CB-5238
https://issues.apache.org/jira/browse/CB-5238
Repository: cordova-plugman
Description
-------
Adding/Removing custom frameworks to iOS projects
node-xcode 0.6.3 needs to be pushed to npm for this to be tested.
Custom frameworks can be defined in plugin.xml like so:
<framework src="src/ios/Custom.framework" custom="true">
if @custom="true" is not defined, plugman will think it is a System framework and will therefore fail to add it properly.
Diffs
-----
spec/platforms/ios.spec.js cb7460d
spec/plugins/ChildBrowser/plugin.xml 512c02f
spec/plugins/DummyPlugin/plugin.xml fad5072
spec/plugins/FaultyPlugin/plugin.xml 30af6e7
spec/util/config-changes.spec.js 5e46b69
src/install.js c0a4de9
src/platforms/ios.js 065c4a7
src/uninstall.js 11ff7fb
src/util/config-changes.js 0239b61
Diff: https://reviews.apache.org/r/15750/diff/
Testing
-------
Thanks,
Anis Kadri
Re: Review Request 15750: Adding/Removing custom frameworks to iOS projects
Posted by Max Woghiren <ma...@chromium.org>.
> On Nov. 21, 2013, 3:44 a.m., Ian Clelland wrote:
> > The documentation should probably be clear that 'custom="false"' will *also* designate a custom framework -- I suspect that some people will try that, rather than simply removing the attribute.
> >
> > Also, what happens if two plugins depend on the same framework? It looks like uninstalling either of them will remove the framework, but I'm not certain.
Will having two plugins depend on the same framework work to begin with? It looks like it will fail with "target destination [path] already exists".
- Max
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15750/#review29208
-----------------------------------------------------------
On Nov. 21, 2013, 3:26 a.m., Anis Kadri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15750/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2013, 3:26 a.m.)
>
>
> Review request for cordova, Max Woghiren and Braden Shepherdson.
>
>
> Bugs: CB-5238
> https://issues.apache.org/jira/browse/CB-5238
>
>
> Repository: cordova-plugman
>
>
> Description
> -------
>
> Adding/Removing custom frameworks to iOS projects
>
> node-xcode 0.6.3 needs to be pushed to npm for this to be tested.
>
> Custom frameworks can be defined in plugin.xml like so:
>
> <framework src="src/ios/Custom.framework" custom="true">
>
> if @custom="true" is not defined, plugman will think it is a System framework and will therefore fail to add it properly.
>
>
> Diffs
> -----
>
> spec/platforms/ios.spec.js cb7460d
> spec/plugins/ChildBrowser/plugin.xml 512c02f
> spec/plugins/DummyPlugin/plugin.xml fad5072
> spec/plugins/FaultyPlugin/plugin.xml 30af6e7
> spec/util/config-changes.spec.js 5e46b69
> src/install.js c0a4de9
> src/platforms/ios.js 065c4a7
> src/uninstall.js 11ff7fb
> src/util/config-changes.js 0239b61
>
> Diff: https://reviews.apache.org/r/15750/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Anis Kadri
>
>
Re: Review Request 15750: Adding/Removing custom frameworks to iOS projects
Posted by Anis Kadri <an...@apache.org>.
> On Nov. 21, 2013, 3:44 a.m., Ian Clelland wrote:
> > The documentation should probably be clear that 'custom="false"' will *also* designate a custom framework -- I suspect that some people will try that, rather than simply removing the attribute.
> >
> > Also, what happens if two plugins depend on the same framework? It looks like uninstalling either of them will remove the framework, but I'm not certain.
>
> Max Woghiren wrote:
> Will having two plugins depend on the same framework work to begin with? It looks like it will fail with "target destination [path] already exists".
>
> Ian Clelland wrote:
> I thought about that as well, (that's why the "what happens if" is there,) but I figured breaking on removal was a more serious bug :)
>
> If we go ahead with this for a v1 implementation, then a solution would just be to have a dedicated plugin to manage the framework, and then plugins depending on the framework would depend on that plugin instead.
>
>
> Max Woghiren wrote:
> Good call—that'll work.
Why does 'custom="false"' also designate a custom framework too?
I agree with the strategy of having a plugin that uses a framework and plugins depend on it. I also think it's an edge case that is not very likely to happen.
- Anis
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15750/#review29208
-----------------------------------------------------------
On Nov. 21, 2013, 3:26 a.m., Anis Kadri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15750/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2013, 3:26 a.m.)
>
>
> Review request for cordova, Max Woghiren and Braden Shepherdson.
>
>
> Bugs: CB-5238
> https://issues.apache.org/jira/browse/CB-5238
>
>
> Repository: cordova-plugman
>
>
> Description
> -------
>
> Adding/Removing custom frameworks to iOS projects
>
> node-xcode 0.6.3 needs to be pushed to npm for this to be tested.
>
> Custom frameworks can be defined in plugin.xml like so:
>
> <framework src="src/ios/Custom.framework" custom="true">
>
> if @custom="true" is not defined, plugman will think it is a System framework and will therefore fail to add it properly.
>
>
> Diffs
> -----
>
> spec/platforms/ios.spec.js cb7460d
> spec/plugins/ChildBrowser/plugin.xml 512c02f
> spec/plugins/DummyPlugin/plugin.xml fad5072
> spec/plugins/FaultyPlugin/plugin.xml 30af6e7
> spec/util/config-changes.spec.js 5e46b69
> src/install.js c0a4de9
> src/platforms/ios.js 065c4a7
> src/uninstall.js 11ff7fb
> src/util/config-changes.js 0239b61
>
> Diff: https://reviews.apache.org/r/15750/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Anis Kadri
>
>
Re: Review Request 15750: Adding/Removing custom frameworks to iOS projects
Posted by Ian Clelland <ic...@chromium.org>.
> On Nov. 21, 2013, 3:44 a.m., Ian Clelland wrote:
> > The documentation should probably be clear that 'custom="false"' will *also* designate a custom framework -- I suspect that some people will try that, rather than simply removing the attribute.
> >
> > Also, what happens if two plugins depend on the same framework? It looks like uninstalling either of them will remove the framework, but I'm not certain.
>
> Max Woghiren wrote:
> Will having two plugins depend on the same framework work to begin with? It looks like it will fail with "target destination [path] already exists".
I thought about that as well, (that's why the "what happens if" is there,) but I figured breaking on removal was a more serious bug :)
If we go ahead with this for a v1 implementation, then a solution would just be to have a dedicated plugin to manage the framework, and then plugins depending on the framework would depend on that plugin instead.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15750/#review29208
-----------------------------------------------------------
On Nov. 21, 2013, 3:26 a.m., Anis Kadri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15750/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2013, 3:26 a.m.)
>
>
> Review request for cordova, Max Woghiren and Braden Shepherdson.
>
>
> Bugs: CB-5238
> https://issues.apache.org/jira/browse/CB-5238
>
>
> Repository: cordova-plugman
>
>
> Description
> -------
>
> Adding/Removing custom frameworks to iOS projects
>
> node-xcode 0.6.3 needs to be pushed to npm for this to be tested.
>
> Custom frameworks can be defined in plugin.xml like so:
>
> <framework src="src/ios/Custom.framework" custom="true">
>
> if @custom="true" is not defined, plugman will think it is a System framework and will therefore fail to add it properly.
>
>
> Diffs
> -----
>
> spec/platforms/ios.spec.js cb7460d
> spec/plugins/ChildBrowser/plugin.xml 512c02f
> spec/plugins/DummyPlugin/plugin.xml fad5072
> spec/plugins/FaultyPlugin/plugin.xml 30af6e7
> spec/util/config-changes.spec.js 5e46b69
> src/install.js c0a4de9
> src/platforms/ios.js 065c4a7
> src/uninstall.js 11ff7fb
> src/util/config-changes.js 0239b61
>
> Diff: https://reviews.apache.org/r/15750/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Anis Kadri
>
>
Re: Review Request 15750: Adding/Removing custom frameworks to iOS projects
Posted by Ian Clelland <ic...@chromium.org>.
> On Nov. 21, 2013, 3:44 a.m., Ian Clelland wrote:
> > The documentation should probably be clear that 'custom="false"' will *also* designate a custom framework -- I suspect that some people will try that, rather than simply removing the attribute.
> >
> > Also, what happens if two plugins depend on the same framework? It looks like uninstalling either of them will remove the framework, but I'm not certain.
>
> Max Woghiren wrote:
> Will having two plugins depend on the same framework work to begin with? It looks like it will fail with "target destination [path] already exists".
>
> Ian Clelland wrote:
> I thought about that as well, (that's why the "what happens if" is there,) but I figured breaking on removal was a more serious bug :)
>
> If we go ahead with this for a v1 implementation, then a solution would just be to have a dedicated plugin to manage the framework, and then plugins depending on the framework would depend on that plugin instead.
>
>
> Max Woghiren wrote:
> Good call—that'll work.
>
> Anis Kadri wrote:
> Why does 'custom="false"' also designate a custom framework too?
> I agree with the strategy of having a plugin that uses a framework and plugins depend on it. I also think it's an edge case that is not very likely to happen.
Um.. that was me looking at src/platforms/ios.js and src/util/config-changes.js, where you only test for the existence of the 'config' attribute, and not its actual value.
I can see that you are using the correct xpath in install.js and uninstall.js, so it will probably not occur in practice. (It looks like only the individual custom="true" elements get passed to the platform handler)
LGTM otherwise
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15750/#review29208
-----------------------------------------------------------
On Nov. 21, 2013, 3:26 a.m., Anis Kadri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15750/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2013, 3:26 a.m.)
>
>
> Review request for cordova, Max Woghiren and Braden Shepherdson.
>
>
> Bugs: CB-5238
> https://issues.apache.org/jira/browse/CB-5238
>
>
> Repository: cordova-plugman
>
>
> Description
> -------
>
> Adding/Removing custom frameworks to iOS projects
>
> node-xcode 0.6.3 needs to be pushed to npm for this to be tested.
>
> Custom frameworks can be defined in plugin.xml like so:
>
> <framework src="src/ios/Custom.framework" custom="true">
>
> if @custom="true" is not defined, plugman will think it is a System framework and will therefore fail to add it properly.
>
>
> Diffs
> -----
>
> spec/platforms/ios.spec.js cb7460d
> spec/plugins/ChildBrowser/plugin.xml 512c02f
> spec/plugins/DummyPlugin/plugin.xml fad5072
> spec/plugins/FaultyPlugin/plugin.xml 30af6e7
> spec/util/config-changes.spec.js 5e46b69
> src/install.js c0a4de9
> src/platforms/ios.js 065c4a7
> src/uninstall.js 11ff7fb
> src/util/config-changes.js 0239b61
>
> Diff: https://reviews.apache.org/r/15750/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Anis Kadri
>
>
Re: Review Request 15750: Adding/Removing custom frameworks to iOS projects
Posted by Max Woghiren <ma...@chromium.org>.
> On Nov. 21, 2013, 3:44 a.m., Ian Clelland wrote:
> > The documentation should probably be clear that 'custom="false"' will *also* designate a custom framework -- I suspect that some people will try that, rather than simply removing the attribute.
> >
> > Also, what happens if two plugins depend on the same framework? It looks like uninstalling either of them will remove the framework, but I'm not certain.
>
> Max Woghiren wrote:
> Will having two plugins depend on the same framework work to begin with? It looks like it will fail with "target destination [path] already exists".
>
> Ian Clelland wrote:
> I thought about that as well, (that's why the "what happens if" is there,) but I figured breaking on removal was a more serious bug :)
>
> If we go ahead with this for a v1 implementation, then a solution would just be to have a dedicated plugin to manage the framework, and then plugins depending on the framework would depend on that plugin instead.
>
Good call—that'll work.
- Max
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15750/#review29208
-----------------------------------------------------------
On Nov. 21, 2013, 3:26 a.m., Anis Kadri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15750/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2013, 3:26 a.m.)
>
>
> Review request for cordova, Max Woghiren and Braden Shepherdson.
>
>
> Bugs: CB-5238
> https://issues.apache.org/jira/browse/CB-5238
>
>
> Repository: cordova-plugman
>
>
> Description
> -------
>
> Adding/Removing custom frameworks to iOS projects
>
> node-xcode 0.6.3 needs to be pushed to npm for this to be tested.
>
> Custom frameworks can be defined in plugin.xml like so:
>
> <framework src="src/ios/Custom.framework" custom="true">
>
> if @custom="true" is not defined, plugman will think it is a System framework and will therefore fail to add it properly.
>
>
> Diffs
> -----
>
> spec/platforms/ios.spec.js cb7460d
> spec/plugins/ChildBrowser/plugin.xml 512c02f
> spec/plugins/DummyPlugin/plugin.xml fad5072
> spec/plugins/FaultyPlugin/plugin.xml 30af6e7
> spec/util/config-changes.spec.js 5e46b69
> src/install.js c0a4de9
> src/platforms/ios.js 065c4a7
> src/uninstall.js 11ff7fb
> src/util/config-changes.js 0239b61
>
> Diff: https://reviews.apache.org/r/15750/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Anis Kadri
>
>
Re: Review Request 15750: Adding/Removing custom frameworks to iOS projects
Posted by Ian Clelland <ic...@chromium.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15750/#review29208
-----------------------------------------------------------
The documentation should probably be clear that 'custom="false"' will *also* designate a custom framework -- I suspect that some people will try that, rather than simply removing the attribute.
Also, what happens if two plugins depend on the same framework? It looks like uninstalling either of them will remove the framework, but I'm not certain.
- Ian Clelland
On Nov. 21, 2013, 3:26 a.m., Anis Kadri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15750/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2013, 3:26 a.m.)
>
>
> Review request for cordova, Max Woghiren and Braden Shepherdson.
>
>
> Bugs: CB-5238
> https://issues.apache.org/jira/browse/CB-5238
>
>
> Repository: cordova-plugman
>
>
> Description
> -------
>
> Adding/Removing custom frameworks to iOS projects
>
> node-xcode 0.6.3 needs to be pushed to npm for this to be tested.
>
> Custom frameworks can be defined in plugin.xml like so:
>
> <framework src="src/ios/Custom.framework" custom="true">
>
> if @custom="true" is not defined, plugman will think it is a System framework and will therefore fail to add it properly.
>
>
> Diffs
> -----
>
> spec/platforms/ios.spec.js cb7460d
> spec/plugins/ChildBrowser/plugin.xml 512c02f
> spec/plugins/DummyPlugin/plugin.xml fad5072
> spec/plugins/FaultyPlugin/plugin.xml 30af6e7
> spec/util/config-changes.spec.js 5e46b69
> src/install.js c0a4de9
> src/platforms/ios.js 065c4a7
> src/uninstall.js 11ff7fb
> src/util/config-changes.js 0239b61
>
> Diff: https://reviews.apache.org/r/15750/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Anis Kadri
>
>
Re: Review Request 15750: Adding/Removing custom frameworks to iOS projects
Posted by Braden Shepherdson <br...@chromium.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15750/#review29231
-----------------------------------------------------------
I share Ian's concern about the removal. Though possibly since these are custom, bundled frameworks, there will never be any overlap. If so, then it's fine.
- Braden Shepherdson
On Nov. 21, 2013, 3:26 a.m., Anis Kadri wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15750/
> -----------------------------------------------------------
>
> (Updated Nov. 21, 2013, 3:26 a.m.)
>
>
> Review request for cordova, Max Woghiren and Braden Shepherdson.
>
>
> Bugs: CB-5238
> https://issues.apache.org/jira/browse/CB-5238
>
>
> Repository: cordova-plugman
>
>
> Description
> -------
>
> Adding/Removing custom frameworks to iOS projects
>
> node-xcode 0.6.3 needs to be pushed to npm for this to be tested.
>
> Custom frameworks can be defined in plugin.xml like so:
>
> <framework src="src/ios/Custom.framework" custom="true">
>
> if @custom="true" is not defined, plugman will think it is a System framework and will therefore fail to add it properly.
>
>
> Diffs
> -----
>
> spec/platforms/ios.spec.js cb7460d
> spec/plugins/ChildBrowser/plugin.xml 512c02f
> spec/plugins/DummyPlugin/plugin.xml fad5072
> spec/plugins/FaultyPlugin/plugin.xml 30af6e7
> spec/util/config-changes.spec.js 5e46b69
> src/install.js c0a4de9
> src/platforms/ios.js 065c4a7
> src/uninstall.js 11ff7fb
> src/util/config-changes.js 0239b61
>
> Diff: https://reviews.apache.org/r/15750/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Anis Kadri
>
>