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 2015/05/25 18:25:23 UTC
[GitHub] cordova-windows pull request: Draft implementation for Unified pla...
GitHub user vladimir-kotikov opened a pull request:
https://github.com/apache/cordova-windows/pull/84
Draft implementation for Unified platform API
Draft implementation of Unified Platform API for Windows platform, that inspired by [this thread](http://markmail.org/thread/3dw4mis4qo5d4ecz) in cordova-dev mailing list and [this proposal](https://github.com/kamrik/cordova-discuss/blob/master/proposals/PlatformProject.md)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-windows unified_platform_api
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-windows/pull/84.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 #84
----
commit 8b6b50212dbfc6aacbf0cdda861c63ff81555295
Author: Vladimir Kotikov <v-...@microsoft.com>
Date: 2015-05-05T10:05:55Z
Adds `requirements` command support to check_reqs module
commit 89b8afa914dba31feb7ab87150cb91e2dae1a54a
Author: Vladimir Kotikov <v-...@microsoft.com>
Date: 2015-05-19T12:12:52Z
Merge remote-tracking branch 'MSOpenTech/CB-8954' into unified_platform_api
commit 7de25c9440b1faf39d315dee0c8792f3c60259ef
Author: Vladimir Kotikov <v-...@microsoft.com>
Date: 2015-05-19T12:27:48Z
Adds basic implementation of PlatformApi for windows
----
---
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-windows pull request: Draft implementation for Unified pla...
Posted by kamrik <gi...@git.apache.org>.
Github user kamrik commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/84#discussion_r31053467
--- Diff: template/cordova/Api.js ---
@@ -0,0 +1,54 @@
+
+/*jshint node: true*/
+
+var path = require('path');
+
+var buildImpl = require('./lib/build');
+var requirementsImpl = require('./lib/check_reqs');
+
+function PlatformApi () {
+ // Set up basic properties. They probably will be overridden if this API is used by cordova-lib
+ this.root = path.join(__dirname, '..', '..');
+ this.platform = 'windows';
+
+ if (this.constructor.super_){
+ // This should only happen if this class is being instantiated from cordova-lib
+ // In this case the arguments is being passed from cordova-lib as well,
+ // so we don't need to care about whether they're correct ot not
+ this.constructor.super_.apply(this, arguments);
+ }
+}
+
+PlatformApi.prototype.build = function(context) {
+ var buildOptions = context && context.options || [];
+ return buildImpl.run(buildOptions);
+};
+
+PlatformApi.prototype.requirements = function () {
+ return requirementsImpl.check_all();
+};
+
+function PlatformHandler() {
--- End diff --
Do we want to keep PlatformHandler and PlatformApi as 2 separate classes long term?
From the current CLI style workflow, the methods in PlatformHandler seem to be like something private that should be hidden from the user. But actually some of the methods like getWwwDir() might be very useful to the user. An example would be when the web app is very large and copying it all on updateWww become a heavy operation. But if the user knows where to copy the files to, he can fine-tune his workflow to only copy changes which is a very common case when working with gulp/grunt watch.
---
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-windows pull request: Draft implementation for Unified pla...
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-windows/pull/84#issuecomment-106845669
I think we should wait for the impending Cordova-windows 4.0 release before we merge this.
---
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-windows pull request: Draft implementation for Unified pla...
Posted by kamrik <gi...@git.apache.org>.
Github user kamrik commented on the pull request:
https://github.com/apache/cordova-windows/pull/84#issuecomment-106447071
Cool, looks much simpler now on the platform side and can let us eventually get rid of the handler when all platform move to use PlatformApi.
---
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-windows pull request: Draft implementation for Unified pla...
Posted by kamrik <gi...@git.apache.org>.
Github user kamrik commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/84#discussion_r31144880
--- Diff: template/cordova/Api.js ---
@@ -0,0 +1,54 @@
+
+/*jshint node: true*/
+
+var path = require('path');
+
+var buildImpl = require('./lib/build');
+var requirementsImpl = require('./lib/check_reqs');
+
+function PlatformApi () {
+ // Set up basic properties. They probably will be overridden if this API is used by cordova-lib
+ this.root = path.join(__dirname, '..', '..');
+ this.platform = 'windows';
+
+ if (this.constructor.super_){
+ // This should only happen if this class is being instantiated from cordova-lib
+ // In this case the arguments is being passed from cordova-lib as well,
+ // so we don't need to care about whether they're correct ot not
+ this.constructor.super_.apply(this, arguments);
+ }
+}
+
+PlatformApi.prototype.build = function(context) {
+ var buildOptions = context && context.options || [];
+ return buildImpl.run(buildOptions);
+};
+
+PlatformApi.prototype.requirements = function () {
+ return requirementsImpl.check_all();
+};
+
+function PlatformHandler() {
--- End diff --
The only reason I kept the parser property when switching to PlatformProject was backwards compatibility as well, with the hope to deprecate it once the rest of the code is changed to use methods of the PlatformProject directly. Actually there are very few direct calls to the parser left in cordova-lib, it's mostly used by the tests (which will have to change anyway).
The "prepare" has several sub-steps which might be interesting to the outside user as separate steps:
* apply plugin config munges
* apply config.xml based config changes to global project files
* copy www
So I'm not sure prepare should be a single black box, but the above steps should definitely be methods of the PlatformApi rather than of some sub-object, and there can be a "prepare" method that executes all 3 of those steps for convenience.
Here is an example usage of a PlatformProject from a gulp file with separate calls to the above 3 stages.
https://github.com/kamrik/cordova-api-example/blob/master/gulpfile.js#L105
The plugin config munges are applied as part of gulp "create" task, and the other two as part of gulp "build" task. This allows for a much faster build for projects with many plugins. But slower for adding/removing plugins (which is a less frequent operation).
Whichever direction you decide to take it, thanks for picking up this API work!
---
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-windows pull request: Draft implementation for Unified pla...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/84#discussion_r31137262
--- Diff: template/cordova/Api.js ---
@@ -0,0 +1,54 @@
+
+/*jshint node: true*/
+
+var path = require('path');
+
+var buildImpl = require('./lib/build');
+var requirementsImpl = require('./lib/check_reqs');
+
+function PlatformApi () {
+ // Set up basic properties. They probably will be overridden if this API is used by cordova-lib
+ this.root = path.join(__dirname, '..', '..');
+ this.platform = 'windows';
+
+ if (this.constructor.super_){
+ // This should only happen if this class is being instantiated from cordova-lib
+ // In this case the arguments is being passed from cordova-lib as well,
+ // so we don't need to care about whether they're correct ot not
+ this.constructor.super_.apply(this, arguments);
+ }
+}
+
+PlatformApi.prototype.build = function(context) {
+ var buildOptions = context && context.options || [];
+ return buildImpl.run(buildOptions);
+};
+
+PlatformApi.prototype.requirements = function () {
+ return requirementsImpl.check_all();
+};
+
+function PlatformHandler() {
--- End diff --
> we need to first move all/most of the code from cordova/metadata/windows_parser to this file before it gets fully operational
This is correct. However, this shouldn't be a problem, because PlatformHandler interface is quite similar to platform parsers in cordova/metadata.
Regarding `PlatformHandler` and `PlatformApi` as 2 separate classes - I like your idea, but I would like to have a bit more feedback on this. The reason why it is implemented this way - i've tried to avoid changes that will break compatibility and obstruct the transition to new Api, so now they acts like former `PlatformProject` and its `parser` property.
BTW, another possible approach here, we're thought about, was removing `parser` property at all and replace it and its methods with single `prepare()` methods which will act more like a "black box" - like `build()`, `run()` and others works now.
---
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-windows pull request: Draft implementation for Unified pla...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:
https://github.com/apache/cordova-windows/pull/84#issuecomment-106307079
@kamrik Thanks a lot for your feedback, i've addressed some of your notes in https://github.com/MSOpenTech/cordova-windows/commit/6a27c27a569ee63894477d58d9593e4bad191e95 and https://github.com/MSOpenTech/cordova-lib/commit/039125e4280a8f3fa37f612aa78bf5d70ddaa220
---
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-windows pull request: Draft implementation for Unified pla...
Posted by kamrik <gi...@git.apache.org>.
Github user kamrik commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/84#discussion_r31052499
--- Diff: template/cordova/Api.js ---
@@ -0,0 +1,54 @@
+
+/*jshint node: true*/
+
+var path = require('path');
+
+var buildImpl = require('./lib/build');
+var requirementsImpl = require('./lib/check_reqs');
+
+function PlatformApi () {
+ // Set up basic properties. They probably will be overridden if this API is used by cordova-lib
+ this.root = path.join(__dirname, '..', '..');
+ this.platform = 'windows';
+
+ if (this.constructor.super_){
+ // This should only happen if this class is being instantiated from cordova-lib
+ // In this case the arguments is being passed from cordova-lib as well,
+ // so we don't need to care about whether they're correct ot not
+ this.constructor.super_.apply(this, arguments);
+ }
+}
+
+PlatformApi.prototype.build = function(context) {
+ var buildOptions = context && context.options || [];
+ return buildImpl.run(buildOptions);
+};
+
+PlatformApi.prototype.requirements = function () {
+ return requirementsImpl.check_all();
+};
+
+function PlatformHandler() {
--- End diff --
Do I understand correctly that we need to first move all/most of the code from cordova/metadata/windows_parser to this file before it gets fully operational?
---
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-windows pull request: Draft implementation for Unified pla...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov closed the pull request at:
https://github.com/apache/cordova-windows/pull/84
---
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