You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by AxelNennker <gi...@git.apache.org> on 2014/07/14 23:41:47 UTC

[GitHub] cordova-lib pull request: CB-7132: fix regression regarding defaul...

GitHub user AxelNennker opened a pull request:

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

    CB-7132: fix regression regarding default resources

    

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

    $ git pull https://github.com/AxelNennker/cordova-lib master

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

    https://github.com/apache/cordova-lib/pull/58.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 #58
    
----
commit 09b16db9df8083c0955680004cb89db23af1459e
Author: ignisvulpis <ax...@nennker.de>
Date:   2014-07-14T21:38:18Z

    CB-7132: fix regression regarding default resources

----


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

[GitHub] cordova-lib pull request: CB-7132: fix regression regarding defaul...

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

    https://github.com/apache/cordova-lib/pull/58#issuecomment-49157473
  
    removed the extra lines. Strange they where there.


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

Re: [GitHub] cordova-lib pull request: CB-7132: fix regression regarding defaul...

Posted by Axel Nennker <ig...@gmail.com>.
Merge artefact.
Am 15.07.2014 03:46 schrieb "kamrik" <gi...@git.apache.org>:

> Github user kamrik commented on a diff in the pull request:
>
>     https://github.com/apache/cordova-lib/pull/58#discussion_r14914123
>
>     --- Diff: cordova-lib/src/cordova/metadata/android_parser.js ---
>     @@ -42,6 +42,12 @@ module.exports = function android_parser(project) {
>          this.android_config = path.join(this.path, 'res', 'xml',
> 'config.xml');
>      };
>
>     +// Returns a promise.
>     +module.exports.check_requirements = function(project_root) {
>     +    // Rely on platform's bin/create script to check requirements.
>     +    return Q(true);
>     +};
>     +
>     --- End diff --
>
>     Is this intentional or a merge artifact?
>     The check_reqs functions were all removed as part of CB-7091
>     commits:
>
> https://github.com/apache/cordova-lib/commit/ab49973560ccb7a1ea37d9f5db4e497ef0ce92d1
>
> https://github.com/apache/cordova-lib/commit/153092f2c235185c7f141979a550779800954a92
>
>
> ---
> 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.
> ---
>

[GitHub] cordova-lib pull request: CB-7132: fix regression regarding defaul...

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

    https://github.com/apache/cordova-lib/pull/58#discussion_r14914123
  
    --- Diff: cordova-lib/src/cordova/metadata/android_parser.js ---
    @@ -42,6 +42,12 @@ module.exports = function android_parser(project) {
         this.android_config = path.join(this.path, 'res', 'xml', 'config.xml');
     };
     
    +// Returns a promise.
    +module.exports.check_requirements = function(project_root) {
    +    // Rely on platform's bin/create script to check requirements.
    +    return Q(true);
    +};
    +
    --- End diff --
    
    Is this intentional or a merge artifact?
    The check_reqs functions were all removed as part of CB-7091
    commits:
    https://github.com/apache/cordova-lib/commit/ab49973560ccb7a1ea37d9f5db4e497ef0ce92d1
    https://github.com/apache/cordova-lib/commit/153092f2c235185c7f141979a550779800954a92


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

Re: [GitHub] cordova-lib pull request: CB-7132: fix regression regarding defaul...

Posted by Axel Nennker <ig...@gmail.com>.
What do you mean with "blank splash"? Splash without src?
If yes then I consider this illegal anyway.

I tested these cases on Android:
- no splash in config.xml
- one default splash. No density.
- all densities that exist in project.
- one density (xhdpi)

All worked.

Axel

PS: I am traveling and am not able to work on this. I think this PR is an
improvement and should be merged even if new bugs were discovered.
Am 18.07.2014 21:26 schrieb "VVelda" <gi...@git.apache.org>:

> Github user VVelda commented on the pull request:
>
>     https://github.com/apache/cordova-lib/pull/58#issuecomment-49475950
>
>     Now it works good with icons. But I have one question, you plain that
> your changes removes splashcreen from project, if in config.xml is blank
> <splash /> tag, don't you?
>     But if I do it and then call cordova prepare, it do, what it shoudl
> do, but also error occurs:
>
>     shell.js: internal error
>     TypeError: Cannot use 'in' operator to search for 'length' in undefined
>         at Object._cp
> (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\cordova-lib\node_modules\shelljs\shell.js:267:26)
>         at Object.cp
> (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\cordova-lib\node_modules\shelljs\shell.js:1491:23)
>         at Object.module.exports.handleSplashes
> (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\cordova-lib\src\cordova\metadata\android_parser.js:115:11)
>         at Object.module.exports.update_from_config
> (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\cordova-lib\src\cordova\metadata\android_parser.js:228:14)
>         at Object.module.exports.update_project
> (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\cordova-lib\src\cordova\metadata\android_parser.js:337:18)
>         at
> C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\cordova-lib\src\cordova\prepare.js:116:31
>         at _fulfilled
> (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\q\q.js:798:54)
>         at self.promiseDispatch.done
> (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\q\q.js:827:30)
>         at Promise.promise.promiseDispatch
> (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\q\q.js:760:13)
>         at
> C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\q\q.js:821:14
>
>     I don't know if it's problem with code, or I'm missing something. (I
> have updated only android-parser.js to test it)
>
>
> ---
> 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.
> ---
>

[GitHub] cordova-lib pull request: CB-7132: fix regression regarding defaul...

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

    https://github.com/apache/cordova-lib/pull/58#issuecomment-49475950
  
    Now it works good with icons. But I have one question, you plain that your changes removes splashcreen from project, if in config.xml is blank <splash /> tag, don't you?
    But if I do it and then call cordova prepare, it do, what it shoudl do, but also error occurs:
    
    shell.js: internal error
    TypeError: Cannot use 'in' operator to search for 'length' in undefined
        at Object._cp (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\cordova-lib\node_modules\shelljs\shell.js:267:26)
        at Object.cp (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\cordova-lib\node_modules\shelljs\shell.js:1491:23)
        at Object.module.exports.handleSplashes (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\cordova-lib\src\cordova\metadata\android_parser.js:115:11)
        at Object.module.exports.update_from_config (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\cordova-lib\src\cordova\metadata\android_parser.js:228:14)
        at Object.module.exports.update_project (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\cordova-lib\src\cordova\metadata\android_parser.js:337:18)
        at C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\cordova-lib\src\cordova\prepare.js:116:31
        at _fulfilled (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\q\q.js:798:54)
        at self.promiseDispatch.done (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\q\q.js:827:30)
        at Promise.promise.promiseDispatch (C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\q\q.js:760:13)
        at C:\Users\Velda\AppData\Roaming\npm\node_modules\cordova\node_modules\q\q.js:821:14
    
    I don't know if it's problem with code, or I'm missing something. (I have updated only android-parser.js to test it)


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

Re: [GitHub] cordova-lib pull request: CB-7132: fix regression regarding defaul...

Posted by Axel Nennker <ig...@gmail.com>.
fixed


2014-07-16 20:29 GMT+02:00 VVelda <gi...@git.apache.org>:

> Github user VVelda commented on the pull request:
>
>     https://github.com/apache/cordova-lib/pull/58#issuecomment-49206968
>
>     Changes deletes also icons and splashscreen in new blank project,
> which is bad. That should not to be done, until is icon or splashreen set
> in config.xml.
>
>
> ---
> 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.
> ---
>

[GitHub] cordova-lib pull request: CB-7132: fix regression regarding defaul...

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

    https://github.com/apache/cordova-lib/pull/58#issuecomment-49206968
  
    Changes deletes also icons and splashscreen in new blank project, which is bad. That should not to be done, until is icon or splashreen set in config.xml.


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

[GitHub] cordova-lib pull request: CB-7132: fix regression regarding defaul...

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

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


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