cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gorkem Ercan <gorkem.er...@gmail.com>
Subject Re: [GitHub] cordova-cli pull request: CB-6469 - Restore plugins from config.xm...
Date Tue, 06 May 2014 15:20:54 GMT
Any updates for me?
--
Gorkem


On Fri, Apr 18, 2014 at 10:21 PM, Michal Mocny <mmocny@chromium.org> wrote:

> On Fri, Apr 18, 2014 at 4:11 PM, Gorkem Ercan <gorkem.ercan@gmail.com
> >wrote:
>
> > On Thu, Apr 17, 2014 at 3:46 PM, Michal Mocny <mmocny@chromium.org>
> wrote:
> >
> > > Took a quick glance.  General questions:
> > > - why the need for save?  Why not just alter the list on each cordova
> > > plugin add/rm?
> > >
> >
> > I do not want to force this workflow. Calling save does not bring much
> > overhead, considering plugin list does not probably change constantly.
> >
>
> If you are going to make this choice, then I would not like to
> automatically install on prepare.  There should be an explicit "load"
> command.  (those names are wrong, but you get the point).
>
> Either we automatically manage plugin installs, or explicitly manage them.
>  I'm happy to start with explicit and support opt-in to automatic handling
> later once we have confidence in the feature.
>
>
> >
> >
> > > - wouldn't "cordova plugin rm foo && cordova prepare" re-install that
> > > plugin right now?
> > >
> >
> > This workflow would require an explicit update to config.xml if a plugin
> is
> > persisted in there. This is a good point, I shall update plugins rm to
> > print a warning about it.
>
>
> >
> > > - why the name <feature> and not <dependency> ?  I think this
> > functionality
> > > should overlap with the plugin.xml spec.
> > >
> > >
> > feature tag lives in the w3c widget spec which has advantages for those
> > (like myself)  who does provide tools for editing config,xml.
> >
>
> We are no longer using that spec, and I as we move more functionality from
> plugins.xml into config.xml we should strive to keep them in line.  It also
> makes our docs easier.
>
>
> >
> >
> >
> > > I haven't put the PR through testing yet.
> > >
> > >
> > > On Thu, Apr 17, 2014 at 5:54 PM, Jesse <purplecabbage@gmail.com>
> wrote:
> > >
> > > > Yeah, that looks weird.
> > > >
> > > > @purplecabbage
> > > > risingj.com
> > > >
> > > >
> > > > On Thu, Apr 17, 2014 at 2:46 PM, kamrik <git@git.apache.org> wrote:
> > > >
> > > > > Github user kamrik commented on a diff in the pull request:
> > > > >
> > > > >
> > > https://github.com/apache/cordova-cli/pull/165#discussion_r11755812
> > > > >
> > > > >     --- Diff: src/save.js ---
> > > > >     @@ -0,0 +1,71 @@
> > > > >     +/**
> > > > >     +    Licensed to the Apache Software Foundation (ASF) under one
> > > > >     +    or more contributor license agreements.  See the NOTICE
> file
> > > > >     +    distributed with this work for additional information
> > > > >     +    regarding copyright ownership.  The ASF licenses this file
> > > > >     +    to you under the Apache License, Version 2.0 (the
> > > > >     +    "License"); you may not use this file except in compliance
> > > > >     +    with the License.  You may obtain a copy of the License
at
> > > > >     +
> > > > >     +    http://www.apache.org/licenses/LICENSE-2.0
> > > > >     +
> > > > >     +    Unless required by applicable law or agreed to in writing,
> > > > >     +    software distributed under the License is distributed on
> an
> > > > >     +    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> > > > >     +    KIND, either express or implied.  See the License for the
> > > > >     +    specific language governing permissions and limitations
> > > > >     +    under the License.
> > > > >     +*/
> > > > >     +
> > > > >     +var cordova_util     = require('./util'),
> > > > >     +    ConfigParser     = require('./ConfigParser'),
> > > > >     +    path             = require('path'),
> > > > >     +    xml              =require('./xml-helpers')
> > > > >     +    Q                = require('q'),
> > > > >     +    events           = require('./events');
> > > > >     +
> > > > >     +;
> > > > >     +
> > > > >     +
> > > > >     +module.exports = function save(target){
> > > > >     +  var projectHome = cordova_util.cdProjectRoot();
> > > > >     +  var configPath = cordova_util.projectConfig(projectHome);
> > > > >     +  var configXml = new ConfigParser(configPath);
> > > > >     +  var pluginsPath = path.join(projectHome, 'plugins');
> > > > >     +  var plugins = cordova_util.findPlugins(pluginsPath);
> > > > >     +
> > > > >     +  return Q.all(plugins.map(function(plugin){
> > > > >     +    var currentPluginPath = path.join(pluginsPath,plugin);
> > > > >     +    var name = readPluginName(currentPluginPath);
> > > > >     +    var id = plugin;
> > > > >     +    var version = readPluginVersion(currentPluginPath);
> > > > >     +    var features = configXml.doc.findall('feature');
> > > > >     +      for(var i=0; i<features.length; i++){
> > > > >     +        if(features[i].attrib.name === name){
> > > > >     +          events.emit('results', 'An entry for "'+ plugin+ '"
> > > > already
> > > > > exists');
> > > > >     +          return Q();
> > > > >     +        }
> > > > >     +      }
> > > > >     +    configXml.addFeature(name, JSON.parse('[{"name":"id",
> > > > > "value":"'+id+'"},{"name":"version", "value":"'+version+'"}]'));
> > > > >     --- End diff --
> > > > >
> > > > >     I might be missing something, but why JSON.parse() rather than
> > just
> > > > > literal array of objects?
> > > > >
> > > > >
> > > > > ---
> > > > > 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.
> > > > > ---
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message