You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by daserge <gi...@git.apache.org> on 2016/04/29 15:28:05 UTC

[GitHub] cordova-windows pull request: CB-11176 Fix windows-splashscreen co...

GitHub user daserge opened a pull request:

    https://github.com/apache/cordova-windows/pull/172

    CB-11176 Fix windows-splashscreen compatibility with older plugin ver…

    …sions
    
    [Jira issue with details](https://issues.apache.org/jira/browse/CB-11176)

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

    $ git pull https://github.com/MSOpenTech/cordova-windows CB-11176

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

    https://github.com/apache/cordova-windows/pull/172.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 #172
    
----
commit 16ad1fd151e1b64631588790d6225b44b5297f1f
Author: daserge <v-...@microsoft.com>
Date:   2016-04-29T13:19:32Z

    CB-11176 Fix windows-splashscreen compatibility with older plugin versions

----


---
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-windows pull request #172: CB-11176 Fix windows-splashscreen compati...

Posted by cadesalaberry <gi...@git.apache.org>.
Github user cadesalaberry commented on a diff in the pull request:

    https://github.com/apache/cordova-windows/pull/172#discussion_r75856167
  
    --- Diff: cordova-js-src/exec.js ---
    @@ -41,6 +41,20 @@ var execProxy = require('cordova/exec/proxy');
      */
     module.exports = function (success, fail, service, action, args) {
     
    +    // Handle the case when we have an old version of splashscreen plugin to avoid the API calls failures
    +    if (service === 'SplashScreen') {
    +        var pluginsVersions = require("cordova/plugin_list").metadata;
    +        var splashscreenVersion = pluginsVersions['cordova-plugin-splashscreen'];
    +        var MIN_SPLASHSCREEN_SUPPORTED_VER = 4;
    --- End diff --
    
    From this line, should I understand that the next 4.x.x release here https://github.com/apache/cordova-plugin-splashscreen/releases will be fully compatible with windows ?


---
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-windows pull request: CB-11176 Fix windows-splashscreen co...

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

    https://github.com/apache/cordova-windows/pull/172#issuecomment-215833144
  
    Minor comments. Otherwise LGTM.


---
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-windows pull request: CB-11176 Fix windows-splashscreen co...

Posted by rakatyal <gi...@git.apache.org>.
Github user rakatyal commented on a diff in the pull request:

    https://github.com/apache/cordova-windows/pull/172#discussion_r61621269
  
    --- Diff: cordova-js-src/exec.js ---
    @@ -41,6 +41,20 @@ var execProxy = require('cordova/exec/proxy');
      */
     module.exports = function (success, fail, service, action, args) {
     
    +    // Handle the case when we have an old version of splashscreen plugin to avoid the API calls failures
    +    if (service === 'SplashScreen') {
    +        var pluginsVersions = require("cordova/plugin_list").metadata;
    +        var splashscreenVersion = pluginsVersions['cordova-plugin-splashscreen'];
    +        var MIN_SPLASHSCREEN_SUPPORTED_VER = 4;
    +        if (splashscreenVersion && parseInt(splashscreenVersion[0], 10) < MIN_SPLASHSCREEN_SUPPORTED_VER) {
    +            console.log('Warning: cordova-plugin-splashscreen has been hooked into for compatibility reasons. Update the plugin to version >= 4.');
    --- End diff --
    
    Consider updating the message to something like: "A more recent version of cordova-plugin-splashscreen has been hooked into..."


---
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-windows pull request: CB-11176 Fix windows-splashscreen co...

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

    https://github.com/apache/cordova-windows/pull/172


---
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-windows pull request: CB-11176 Fix windows-splashscreen co...

Posted by rakatyal <gi...@git.apache.org>.
Github user rakatyal commented on a diff in the pull request:

    https://github.com/apache/cordova-windows/pull/172#discussion_r61619913
  
    --- Diff: cordova-js-src/exec.js ---
    @@ -41,6 +41,20 @@ var execProxy = require('cordova/exec/proxy');
      */
     module.exports = function (success, fail, service, action, args) {
     
    +    // Handle the case when we have an old version of splashscreen plugin to avoid the API calls failures
    +    if (service === 'SplashScreen') {
    +        var pluginsVersions = require("cordova/plugin_list").metadata;
    +        var splashscreenVersion = pluginsVersions['cordova-plugin-splashscreen'];
    +        var MIN_SPLASHSCREEN_SUPPORTED_VER = 4;
    +        if (splashscreenVersion && parseInt(splashscreenVersion[0], 10) < MIN_SPLASHSCREEN_SUPPORTED_VER) {
    --- End diff --
    
    Is it supposed to not work for 4.x versions as well? 


---
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-windows pull request: CB-11176 Fix windows-splashscreen co...

Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on a diff in the pull request:

    https://github.com/apache/cordova-windows/pull/172#discussion_r62048444
  
    --- Diff: cordova-js-src/exec.js ---
    @@ -41,6 +41,20 @@ var execProxy = require('cordova/exec/proxy');
      */
     module.exports = function (success, fail, service, action, args) {
     
    +    // Handle the case when we have an old version of splashscreen plugin to avoid the API calls failures
    +    if (service === 'SplashScreen') {
    +        var pluginsVersions = require("cordova/plugin_list").metadata;
    +        var splashscreenVersion = pluginsVersions['cordova-plugin-splashscreen'];
    +        var MIN_SPLASHSCREEN_SUPPORTED_VER = 4;
    +        if (splashscreenVersion && parseInt(splashscreenVersion[0], 10) < MIN_SPLASHSCREEN_SUPPORTED_VER) {
    --- End diff --
    
    This regards to the plugin version and not to the platform' one,  i.e. it should work starting with cordova-plugin-splashscreen@4 (as it will just redirect API calls to the platform).


---
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-windows pull request: CB-11176 Fix windows-splashscreen co...

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

    https://github.com/apache/cordova-windows/pull/172#issuecomment-216891242
  
    ## [Current coverage][cc-pull] is **73.42%**
    > Merging [#172][cc-pull] into [master][cc-base-branch] will not change coverage
    
    ```diff
    @@             master       #172   diff @@
    ==========================================
      Files            14         14          
      Lines          1862       1862          
      Methods         342        342          
      Messages          0          0          
      Branches        379        379          
    ==========================================
      Hits           1367       1367          
      Misses          495        495          
      Partials          0          0          
    ```
    
    
    
    > Powered by [Codecov](https://codecov.io?src=pr). Last updated by [033956d...b48e5d8][cc-compare]
    [cc-base-branch]: https://codecov.io/gh/apache/cordova-windows/branch/master?src=pr
    [cc-compare]: https://codecov.io/gh/apache/cordova-windows/compare/033956d1f561a0d3321199115d7b556ea6ed7d8e...b48e5d8b12a8656b1ddf3a1bd35d86d104d701a2
    [cc-pull]: https://codecov.io/gh/apache/cordova-windows/pull/172?src=pr


---
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-windows pull request: CB-11176 Fix windows-splashscreen co...

Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on a diff in the pull request:

    https://github.com/apache/cordova-windows/pull/172#discussion_r62049082
  
    --- Diff: cordova-js-src/exec.js ---
    @@ -41,6 +41,20 @@ var execProxy = require('cordova/exec/proxy');
      */
     module.exports = function (success, fail, service, action, args) {
     
    +    // Handle the case when we have an old version of splashscreen plugin to avoid the API calls failures
    +    if (service === 'SplashScreen') {
    +        var pluginsVersions = require("cordova/plugin_list").metadata;
    +        var splashscreenVersion = pluginsVersions['cordova-plugin-splashscreen'];
    +        var MIN_SPLASHSCREEN_SUPPORTED_VER = 4;
    +        if (splashscreenVersion && parseInt(splashscreenVersion[0], 10) < MIN_SPLASHSCREEN_SUPPORTED_VER) {
    +            console.log('Warning: cordova-plugin-splashscreen has been hooked into for compatibility reasons. Update the plugin to version >= 4.');
    --- End diff --
    
    Updated, thanks.


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