cordova-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <>
Subject [jira] [Commented] (CB-8627) 'cordova plugin add git_url' erroneously updates fetch.json even when the 'add operation fails'
Date Fri, 05 Jun 2015 23:53:00 GMT


ASF GitHub Bot commented on CB-8627:

Github user TimBarham commented on the pull request:
    As I general design point, I don't like the idea of breaking the `save_fetch_metadata()`
call out so it needs to be called separately, for two reasons:
    1. First and most importantly, `plugman.fetch()` is a public API, and this changes its
behavior (it no longer writes to `fetch.json`, and it returns a different value).
    2. You've tightened the coupling between `cordova.plugin.add` and `plugman` - `cordova.plugin.add`
now needs to know stuff about that result object, and needs to use it to call `save_fetch_metadata()`.
    I think a better approach would be for `fetch()` to support an (*optional*) method that
it calls before it writes `fetch.json` - this method would return a promise (or reject if
something fails). Let's say this parameter was called `validate`, the end of `fetch()` might
look something like this (though you'd need to handle the fact that this method would need
to be optional):
    ``` js
            }).then(function(result) {
                options.plugin_src_dir = result.pinfo.dir;
                return Q.when(copyPlugin(result.pinfo, plugins_dir, &&
result.fetchJsonSource.type == 'local'))
                .then(function(dir) {
                    result.dest = dir;
                    return result;
        }).then(function(result) {
            checkID(options.expected_id, result.pinfo);
            return result;
        }).then(validate).then(function (result) {
            var data = { source: result.fetchJsonSource };
            data.is_top_level = options.is_top_level; 
            data.variables = options.variables || {};
            metadata.save_fetch_metadata(plugins_dir,, data);
            return result.dest;
    Some additional points:
    * In the actual implementation, you should find a cleaner way to pass `result` through
to the final piece of code, rather than requiring the `validate()` method return it like my
example would. Maybe `validate()` doesn't need to be asynchronous (that is, it could just
return a Boolean rather than a promise), but it is certainly more flexible if it supports
it being async.
    * Since `plugman.fetch()` is responsible for creating the plugin's folder and its contents,
it should also be responsible for removing it if the process fails (rather than `plugin.add()`
doing it as happens now).

> 'cordova plugin add git_url' erroneously updates fetch.json even when the 'add operation
> -----------------------------------------------------------------------------------------------
>                 Key: CB-8627
>                 URL:
>             Project: Apache Cordova
>          Issue Type: Bug
>          Components: CLI
>            Reporter: Omar Mefire
>            Assignee: Omar Mefire
> - cordova plugin add
>     This results in fetch.json being updated with the plugin info even though the plugin
installation failed. It should not be the case.

This message was sent by Atlassian JIRA

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message