Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(aggregate): add more info on aggregation changes to upgrade guide #1638

Merged
merged 1 commit into from
Jan 19, 2018

Conversation

daprahamian
Copy link
Contributor

We now consolidate all notes on aggregation in to a single location
in the upgrade guide. We also add a note about how aggregate no
longer accepts variadic arguments.

Fixes NODE-1285

@daprahamian daprahamian requested review from mbroadst and jlord January 16, 2018 21:03
CHANGES_3.0.0.md Outdated
### `aggregate`

`Collection.prototype.aggregate` no longer accepts variadic arguments. Pipeline stages
are now only accepted as an `Array` of stages as the first argument.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be helpful to show before and after images of the input shape here, make it very easy for people to see the difference. Also I think it might pay to explain our rationale for making the change (maintainability, consistency of api, etc)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably worth noting that things like this might have been included for shell compatibility

CHANGES_3.0.0.md Outdated

`Collection.prototype.aggregate` now returns a cursor if a callback is provided. It used to return
the resulting documents which is the same as calling `cursor.toArray()` on the cursor we now pass to
the callback.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto here

CHANGES_3.0.0.md Outdated
@@ -177,7 +173,14 @@ in on the options object . Additionally, `find` does not support individual opti
`limit` as positional parameters. You must either pass in these parameters in the `options` object,
or add them via `Cursor` methods like `Cursor.prototype.skip`.

### Aggregation
### `aggregate`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Collection.prototype.aggregate ? Or just Aggregation? I'm not too picky though

@daprahamian
Copy link
Contributor Author

Updated with feedback from @mbroadst

CHANGES_3.0.0.md Outdated
{$project: {b: 1, _id: 0}}
],
function (err, cursor) {
console.log(cursor.toArray());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toArray is actually an async function

We now consolidate all notes on aggregation in to a single location
in the upgrade guide. We also add a note about how `aggregate` no
longer accepts variadic arguments.

Fixes NODE-1285
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants