You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by MariaBukharina <gi...@git.apache.org> on 2014/10/29 15:53:20 UTC

[GitHub] cordova-plugin-contacts pull request: CB-7896 Pending tests for Sa...

GitHub user MariaBukharina opened a pull request:

    https://github.com/apache/cordova-plugin-contacts/pull/49

    CB-7896 Pending tests for Save and Find methods for windows 

     Pending tests for Save and Find methods for windows cause they are not supported yet

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

    $ git pull https://github.com/MSOpenTech/cordova-plugin-contacts CB-7896

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

    https://github.com/apache/cordova-plugin-contacts/pull/49.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 #49
    
----

----


---
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-plugin-contacts pull request: CB-7896 Pending tests for Sa...

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

    https://github.com/apache/cordova-plugin-contacts/pull/49#issuecomment-64156514
  
    Merged to master


---
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-plugin-contacts pull request: CB-7896 Pending tests for Sa...

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

    https://github.com/apache/cordova-plugin-contacts/pull/49#issuecomment-61074069
  
    This PR is targeting Windows platform. Windows phone 8 is out of scope


---
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-plugin-contacts pull request: CB-7896 Pending tests for Sa...

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

    https://github.com/apache/cordova-plugin-contacts/pull/49#issuecomment-61308935
  
    For now I've made changes only for “Windows” platform. The line you’ve mentioned corresponds to other platform: “Windows Phone 8”. I'm not changing anything unrelated to “Windows” platform otherwise I would have to test this PR on other platforms as well, which is out of scope for this PR.
    
    Later on hopefully I will review tests for “Windows Phone 8” platform. If at that time this PR won't be merged, I'll just add new commit to this PR (to avoid merging conflicts). Otherwise I'll create another PR for “Windows Phone 8” 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-plugin-contacts pull request: CB-7896 Pending tests for Sa...

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

    https://github.com/apache/cordova-plugin-contacts/pull/49#issuecomment-62334665
  
    You may say this is out of scope of this PR but I would prefer platform check method like isPlatform(["platform1", "platform2", "platform3"]) so that the method can be applied to any 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-plugin-contacts pull request: CB-7896 Pending tests for Sa...

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

    https://github.com/apache/cordova-plugin-contacts/pull/49#issuecomment-65168520
  
    I believe this has been merged in, please close the 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-plugin-contacts pull request: CB-7896 Pending tests for Sa...

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

    https://github.com/apache/cordova-plugin-contacts/pull/49


---
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-plugin-contacts pull request: CB-7896 Pending tests for Sa...

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

    https://github.com/apache/cordova-plugin-contacts/pull/49#issuecomment-62753473
  
    Thanks, I'll consider it when reviewing tests for windows phone 8.


---
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-plugin-contacts pull request: CB-7896 Pending tests for Sa...

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

    https://github.com/apache/cordova-plugin-contacts/pull/49#issuecomment-61225476
  
    Ok, but where is the corresponding WP8 PR? How would you merge this in this case? Commits/issue could be separate but I propose to have a single PR so that there is no merge conflict


---
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-plugin-contacts pull request: CB-7896 Pending tests for Sa...

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

    https://github.com/apache/cordova-plugin-contacts/pull/49#discussion_r20774412
  
    --- Diff: tests/tests.js ---
    @@ -24,8 +24,9 @@ exports.defineAutoTests = function () {
       // all of the setup/teardown test methods can reference the following variables to make sure to do the right cleanup
       var gContactObj = null,
         gContactId = null,
    -    isWindowsPhone = cordova.platformId == 'windowsphone';
    -
    +    isWindowsPhone8 = cordova.platformId == 'windowsphone',
    +    isWindows = (cordova.platformId === "windows") || (navigator.appVersion.indexOf("MSAppHost/1.0") !== -1),
    +    isWindowsPhone81 = (navigator.appVersion.indexOf("Windows Phone 8.1;") !== -1);
    --- End diff --
    
    isWindows = (cordova.platformId === "windows") || (cordova.platformId === "windows8")
    isWindowsPhone81 =  isWIndows && WinJS.Utilities.isPhone


---
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-plugin-contacts pull request: CB-7896 Pending tests for Sa...

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

    https://github.com/apache/cordova-plugin-contacts/pull/49#discussion_r19587554
  
    --- Diff: tests/tests.js ---
    @@ -99,7 +110,14 @@ exports.defineAutoTests = function () {
                   afterEach(removeContact);
     
                   it("contacts.spec.6 should be able to find a contact by name", function (done) {
    -                  if (isWindowsPhone) {
    +                  // Find method is not supported on Windows Store apps.
    +                  // also this test will be skipped for Windows Phone 8.1 because function "save" not supported on WP8.1
    +                  if (isWindows) {
    +                      pending();
    +                      return;
    +                  }
    +
    +                  if (isWindowsPhone8) {
    --- End diff --
    
    Why done() w/o running any tests?


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