You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by vladimir-kotikov <gi...@git.apache.org> on 2016/04/08 14:10:29 UTC
[GitHub] cordova-lib pull request: CB-11022 Improve performance of `cordova...
GitHub user vladimir-kotikov opened a pull request:
https://github.com/apache/cordova-lib/pull/423
CB-11022 Improve performance of `cordova plugin add` command
This addresses [CB-11022](https://issues.apache.org/jira/browse/CB-11022) and dramaticaly reduces time required for plugin installation (especially for the projects with a lot of www assets) by skipping prepare after each plugin install.
This change also requires a corresponding update for the platforms that implement PlatformApi contract - they have to take care of installing (or removing) their js and www files into both `www` and `platform_www` directories and return any non-falsy value as a result of `addPlugin`/`removePlugin` methods
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-lib CB-11022
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-lib/pull/423.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #423
----
commit 161e5fd0f1ba3fe88ad74b1bd30182f91d7b8550
Author: Vladimir Kotikov <v-...@microsoft.com>
Date: 2016-04-07T10:56:49Z
CB-11022 Respect result returned by plugin installation and skip prepare if it is truthy
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11022 Improve performance of `cordova...
Posted by jasongin <gi...@git.apache.org>.
Github user jasongin commented on the pull request:
https://github.com/apache/cordova-lib/pull/423#issuecomment-209620711
Yes, LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11022 Improve performance of `cordova...
Posted by jasongin <gi...@git.apache.org>.
Github user jasongin commented on the pull request:
https://github.com/apache/cordova-lib/pull/423#issuecomment-208603032
OK, I didn't realize config file transformations were also done already in plugin add. And I think I have a better understanding of the code now.
My understanding is we still need to do prepare for any platforms which don't do the following things:
1. Implement the PlatformApi
2. Copy over the plugins' www files in addPlugin()
3. Return true from addPlugin()
So do we also need changes to iOS and Windows platforms to do 2 and 3, to match what you did for Android in the linked PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11022 Improve performance of `cordova...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-lib/pull/423
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11022 Improve performance of `cordova...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:
https://github.com/apache/cordova-lib/pull/423#issuecomment-208828579
> do we also need changes to iOS and Windows platforms
Exactly. Changes for windows and iOS is on their go
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11022 Improve performance of `cordova...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:
https://github.com/apache/cordova-lib/pull/423#issuecomment-207411302
@jasongin, @nikhilkh, please take a look when you have a chance
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11022 Improve performance of `cordova...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:
https://github.com/apache/cordova-lib/pull/423#issuecomment-208311472
The config file transformations are already handled during both plugin add/rm and prepare. Consider the following code: [Api.js#L233-L237](https://github.com/apache/cordova-android/blob/master/bin/templates/cordova/Api.js#L233-L237). It calls `add_plugin_changes` from [ConfigChanges.js#L128](https://github.com/apache/cordova-lib/blob/master/cordova-common/src/ConfigChanges/ConfigChanges.js#L128) which:
1. saves these transormations into `<platform>.json` for further usage and
2. applies these transformations immediately (see [ConfigChanges.js#L160](https://github.com/apache/cordova-lib/blob/master/cordova-common/src/ConfigChanges/ConfigChanges.js#L160)).
So right now the only goal of calling `prepare` right after `plugin add` is to copy www files.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11022 Improve performance of `cordova...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:
https://github.com/apache/cordova-lib/pull/423#issuecomment-209607020
@jasongin, i've added tests to cover the new logic. Is this PR ready to go?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11022 Improve performance of `cordova...
Posted by jasongin <gi...@git.apache.org>.
Github user jasongin commented on the pull request:
https://github.com/apache/cordova-lib/pull/423#issuecomment-207527874
This code is pretty hard to follow. I don't mean your changes, but plugin installation overall is more complex than I expected.
How do these changes affect plugins' config file transformations? Those are normally processed during prepare, right? How would we ever fully eliminate prepare after installing plugins. Don't we need to keep at least the config file transformations?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org