cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From TimBarham <...@git.apache.org>
Subject [GitHub] cordova-windows pull request: CB-8954 Adds support for `requiremen...
Date Tue, 26 May 2015 19:31:36 GMT
Github user TimBarham commented on a diff in the pull request:

    https://github.com/apache/cordova-windows/pull/83#discussion_r31070612
  
    --- Diff: bin/lib/check_reqs.js ---
    @@ -17,17 +17,101 @@
            under the License.
     */
     
    -var Q     = require('Q'),
    -    os    = require('os'),
    +/*jshint node:true*/
    +
    +var Q     = require('Q');
    +
    +var MSBuildTools;
    +try {
         MSBuildTools = require('../../template/cordova/lib/MSBuildTools');
    +} catch (ex) {
    +    // If previous import fails, we probably running this script
    +    // from installed platform and the module location is different.
    +    MSBuildTools = require('./MSBuildTools');
    +}
    +
    +/**
    + * Check if current OS is supports building windows platform
    + * @return {Promise} Promise either fullfilled or rejected with error message.
    + */
    +var checkOS = function () {
    +    var platform = process.platform;
    +    return (platform === 'win32') ?
    +        Q.resolve(platform):
    +        // Build Universal windows apps available for windows platform only, so we reject
on others platforms
    +        Q.reject('Cordova tooling for Windows requires Windows OS to build project');
    +};
    +
    +/**
    + * Checks if MSBuild tools is available.
    + * @return {Promise} Promise either fullfilled with MSBuild version
    + *                           or rejected with error message.
    + */
    +var checkMSBuild = function () {
    +    return MSBuildTools.findAvailableVersion()
    +    .then(function (msbuildTools) {
    +        // return Q.resolve('MSBuild tools v.' + msbuildTools.version + ' found at '
+ msbuildTools.path);
    +        return Q.resolve(msbuildTools.version);
    +    }, function () {
    +        return Q.reject('MSBuild tools not found. Please install MSBuild tools or VS
2013 from ' +
    +            'https://www.visualstudio.com/downloads/download-visual-studio-vs');
    +    });
    +};
     
     module.exports.run = function () {
    -    if (os.platform() != 'win32'){
    -      // Build Universal windows apps available for windows platform only, so we reject
on others platforms
    -        return Q.reject('ERROR: Cordova tooling for Windows requires Windows OS');
    -    }
    -    // Check whther MSBuild Tools are available
    -    return MSBuildTools.findAvailableVersion();
    +    return checkOS().then(function () {
    +        return MSBuildTools.findAvailableVersion();
    +    });
    +};
    +
    +/**
    + * Object thar represents one of requirements for current platform.
    + * @param {String} id         The unique identifier for this requirements.
    + * @param {String} name       The name of requirements. Human-readable field.
    + * @param {String} version    The version of requirement installed. In some cases could
be an array of strings
    + *                            (for example, check_android_target returns an array of
android targets installed)
    + * @param {Boolean} installed Indicates whether the reuirement is installed or not
    + */
    +var Requirement = function (id, name, version, installed) {
    +    this.id = id;
    +    this.name = name;
    +    this.installed = installed || false;
    +    this.metadata = {
    +        version: version,
    +    };
    +};
    +
    +/**
    + * Methods that runs all checks one by one and returns a result of checks
    + * as an array of Requirement objects. This method intended to be used by cordova-lib
check_reqs method
    + *
    + * @return Promise<Requirement[]> Array of requirements. Due to implementation,
promise is always fulfilled.
    + */
    +module.exports.check_all = function() {
    +
    +    var requirements = [
    +        new Requirement('os', 'Windows OS'),
    +        new Requirement('msbuild', 'MSBuild Tools')
    +    ];
    +
    +    // Define list of checks needs to be performed
    +    var checkFns = [checkOS, checkMSBuild];
    +    // Then execute them one-by-one
    +    return checkFns.reduce(function (promise, checkFn, idx) {
    +        // Update each requirement with results
    +        var requirement = requirements[idx];
    +        return promise.then(checkFn)
    +        .then(function (version) {
    +            requirement.installed = true;
    +            requirement.metadata.version = version;
    +        }, function (err) {
    +            requirement.metadata.reason = err;
    +        });
    +    }, Q())
    +    .then(function () {
    +        // When chain is completed, return requirements array to upstream API
    +        return requirements;
    +    });
    --- End diff --
    
    I know it's not a lot of code, but since it will be duplicated for every platform... it
would be nice if the definition of `Requirement`, and the `check_all` logic could be in its
own module that the platforms take as a dependency. A pain to set up now, but easier maintenance
later on.


---
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