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/08/21 17:16:55 UTC

[GitHub] cordova-lib pull request: Adds implementation for PlatformApiPoly ...

GitHub user vladimir-kotikov opened a pull request:

    https://github.com/apache/cordova-lib/pull/282

    Adds implementation for PlatformApiPoly class according to PlatformApi spec

    This implements PlatformApiPoly class according to PlatformApi spec, which allows to:
      * create/update platform
      * execute platform's actions (build/run/add/update)
      * do a prepare (needed for CLI workflow only)
      * install/uninstall plugins
    
    Other noticeable changes:
      * removes `getPlatformProject` and PlatformProject method/class in favor of PlatformApiPoly/getPlatformApi
      * make assets and js-modules installing/uninstalling through ActionStack
      * refactor configChanges to not require plugins_dir in constructor
      * moves mergeXml helper to xml-helpers
    
    This should be used along with cordova-cli/platformApi branch

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/MSOpenTech/cordova-lib PlatformApi

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cordova-lib/pull/282.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 #282
    
----
commit c93124c045a4a816fdcb5921e6e58dd667b60c95
Author: Vladimir Kotikov <v-...@microsoft.com>
Date:   2015-07-10T12:37:54Z

    Initial Implementation of PlatformApiPoly
    
    This implements PlatformApiPoly class according to PlatformApi spec, which allows to:
      * create/update platform
      * execute platform's actions (build/run/add/update)
      * do a prepare (needed for CLI workflow only)
      * install/uninstall plugins
    
    Other noticeable changes:
      * removes `getPlatformProject` and PlatformProject method/class in favor of PlatformApiPoly/getPlatformApi
      * make assets and js-modules installing/uninstalling through ActionStack
      * refactor configChanges to not require plugins_dir in constructor
      * moves mergeXml helper to xml-helpers
    
    This should be used along with cordova-cli/platformApi branch

----


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


RE: [GitHub] cordova-lib pull request: Adds implementation for PlatformApiPoly ...

Posted by "Vladimir Kotikov (Akvelon)" <v-...@microsoft.com>.
Sure, Jesse.

Best regards, Vladimir.

-----Original Message-----
From: Jesse [mailto:purplecabbage@gmail.com] 
Sent: Wednesday, August 26, 2015 3:58 AM
To: dev@cordova.apache.org
Subject: Re: [GitHub] cordova-lib pull request: Adds implementation for PlatformApiPoly ...

This looks great, but I would like to spend a little more time before we push it to master.
Can we hold off until the end of the week?


My team is hiring!
@purplecabbage
https://na01.safelinks.protection.outlook.com/?url=risingj.com&data=01%7c01%7cv-vlkoti%40064d.mgd.microsoft.com%7c828d73bbfb0040b208ca08d2adb17adc%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=82e0uXsUaTTV%2bulynRSm3qAvSwui0W%2bqq2yihY9qMj0%3d

On Tue, Aug 25, 2015 at 5:46 PM, stevengill <gi...@git.apache.org> wrote:

> Github user stevengill commented on the pull request:
>
>     
> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithu
> b.com%2fapache%2fcordova-lib%2fpull%2f282%23issuecomment-134780081&dat
> a=01%7c01%7cv-vlkoti%40064d.mgd.microsoft.com%7c828d73bbfb0040b208ca08
> d2adb17adc%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=SE15OBPRL%2fqm
> ViOBsL0P0yvzKFA5bqAVOkHtPkfDYKI%3d
>
>     Looks great Vlad!  I'm fine with you merging this in and us 
> spending some time testing it on master to make sure it works with all 
> of our platforms.
>
>
> ---
> 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
>
>

Re: [GitHub] cordova-lib pull request: Adds implementation for PlatformApiPoly ...

Posted by Jesse <pu...@gmail.com>.
This looks great, but I would like to spend a little more time before we
push it to master.
Can we hold off until the end of the week?


My team is hiring!
@purplecabbage
risingj.com

On Tue, Aug 25, 2015 at 5:46 PM, stevengill <gi...@git.apache.org> wrote:

> Github user stevengill commented on the pull request:
>
>     https://github.com/apache/cordova-lib/pull/282#issuecomment-134780081
>
>     Looks great Vlad!  I'm fine with you merging this in and us spending
> some time testing it on master to make sure it works with all of our
> platforms.
>
>
> ---
> 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-lib pull request: Adds implementation for PlatformApiPoly ...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on the pull request:

    https://github.com/apache/cordova-lib/pull/282#issuecomment-134780081
  
    Looks great Vlad!  I'm fine with you merging this in and us spending some time testing it on master to make sure it works with all of our platforms. 


---
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-lib pull request: CB-9597 Adds implementation for Platform...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cordova-lib/pull/282


---
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-lib pull request: Adds implementation for PlatformApiPoly ...

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-lib/pull/282#discussion_r37758129
  
    --- Diff: cordova-lib/src/plugman/util/config-changes.js ---
    @@ -168,22 +167,18 @@ function remove_plugin_changes(plugin_name, plugin_id, is_top_level) {
         }
     
         // Remove from installed_plugins
    -    if (is_top_level) {
    -        delete platform_config.installed_plugins[plugin_id];
    -    } else {
    -        delete platform_config.dependent_plugins[plugin_id];
    -    }
    +    self.platformJson.addPlugin(pluginInfo.id, is_top_level);
    --- End diff --
    
    FIXME: probably should be `removePlugin()`


---
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-lib pull request: Adds implementation for PlatformApiPoly ...

Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:

    https://github.com/apache/cordova-lib/pull/282#issuecomment-134241500
  
    Thanks for advice, Mark. I have some plans about tests update and definitely will try fixtures-based approach.


---
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-lib pull request: Adds implementation for PlatformApiPoly ...

Posted by kamrik <gi...@git.apache.org>.
Github user kamrik commented on the pull request:

    https://github.com/apache/cordova-lib/pull/282#issuecomment-134217638
  
    Looks good! and very clean.
    
    One note about tests, jasmine spies seem to give you a lot of trouble. In the previous refactoring round I was doing, I found it very useful to entirely replace some tests that rely on spies too much with something based on filesystem fixtures with less spying.


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