cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From vladimir-kotikov <...@git.apache.org>
Subject [GitHub] cordova-windows pull request: Draft implementation for Unified pla...
Date Wed, 27 May 2015 14:15:52 GMT
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


Mime
View raw message