Skip to content
This repository has been archived by the owner on Dec 12, 2018. It is now read-only.

Collection resource update #591

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

mdeggies
Copy link
Member

@mdeggies mdeggies commented Jan 6, 2017

Fixed broken collection resource method examples and removed docs for the methods that don't exist (collection.concatLimit(), collection.detectLimit(), collection.everyLimit(), collection.everySeries(), collection.filterLimit(),collection.mapLimit(), collection.rejectLimit(), collection.someLimit(), collection.someSeries(),collection.sortByLimit(), collection.sortBySeries()).

One thing to note, there are still some methods remaining that don't work as expected (all of the limit() methods, like collection.eachLimit() and collection.mapLimit()). The docs say they're supposed to be exactly like their parent methods but with an iterator, so for ex, eachLimit should look like this:

    function iterator(account, next) {
      console.log('Found account for ' + account.givenName + ' (' + account.email + ')');
      next();
    }

    function doneCallback(err) {
      if (!err) {
        console.log('All accounts have been visited.');
      }
    }

    application.getAccounts({ email: '[email protected]' }, function (err, collection) {
      if (!err) {
        collection.eachLimit(iterator, 2, doneCallback);
      }
    });

When I run this, though, it doesn't make it into any of the functions (iterator or doneCallback). Please take a look and let me know if they should be fixed here.

@the-overengineer
Copy link

@mdeggies Sorry to not have reacted sooner (I was on vacation at the time this issue was raised), but we already have a PR that solves this issue, the other way around. See #570

The issue is that the documentation uses async v2, while the code uses async v1. The references PR fixes that, updating the lib to async v2 as a dependency. However, it's a breaking change, so it's going into 1.0.0..

Since this is a broader change (additional documentation changes are contained within), I'm just putting this comment here for reference, as the documentation will have to be reverted back to async v2 in the referenced PR if this is merged.

@mdeggies
Copy link
Member Author

@Tweety-FER ahh no problem! I'll leave this to @robertjd or Robin to see what they want to do. Just wanted to clean this up for the time being as we were getting a lot of questions about these methods not working as documented. If we don't merge, I think it would be good to at least add a note to the top of the page explaining the issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants