Skip to content

Conversation

addaleax
Copy link
Contributor

@addaleax addaleax commented Jun 25, 2021

Description

What changed?

Cherry-pick the changes from:

(because a full update didn’t seem feasible at this point)
and update the code and non-spec tests to match the relaxed
naming restrictions.

Are there any files to ignore?

Spec file changes, I assume.

const collection = entities.getEntity('collection', operation.object);
const { filter, replacement, ...opts } = operation.arguments;
return collection.findOneAndReplace(filter, replacement, translateOptions(opts));
return (await collection.findOneAndReplace(filter, replacement, translateOptions(opts))).value;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This matches what findOneAndUpdate does below

@dariakp dariakp added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Jun 25, 2021
@dariakp dariakp self-assigned this Jun 25, 2021
@dariakp dariakp requested a review from nbbeeken June 25, 2021 19:14
@dariakp dariakp assigned nbbeeken and unassigned dariakp Jun 25, 2021
@dariakp
Copy link
Contributor

dariakp commented Jun 25, 2021

Also, let's make sure that any legitimate updates to the unified spec runner get ported to 3.6/3.7

@addaleax
Copy link
Contributor Author

Also, let's make sure that any legitimate updates to the unified spec runner get ported to 3.6/3.7

@dariakp Since I have fairly little knowledge of what exactly that means, please let me know if there is something to do for me here :)

@dariakp
Copy link
Contributor

dariakp commented Jun 25, 2021

Also, let's make sure that any legitimate updates to the unified spec runner get ported to 3.6/3.7

@dariakp Since I have fairly little knowledge of what exactly that means, please let me know if there is something to do for me here :)

This is just a housekeeping note for the team to either open a jira ticket or create a PR for the 3.6 or 3.7 branch in order to keep the unified runner implementations in sync once we determine what changes are needed, nothing for you to worry about @addaleax :)

Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

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

Just the note about the ticket comment, lgtm otherwise! (Made a ticket about the backport needed for the unified runner here NODE-3387)

@nbbeeken nbbeeken added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Jun 25, 2021
@nbbeeken nbbeeken requested review from emadum and durran June 25, 2021 20:35
Copy link
Contributor

@emadum emadum left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

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

mongodb/specifications@fc21cb7
We have a fix in the specs for the insertedCount if you could pull in the latest please and thanks!

Cherry-pick the changes from:

- mongodb/specifications@a124e21
- mongodb/specifications@851ca10
- mongodb/specifications@fc21cb7

(because a full update didn’t seem feasible at this point)
and update the code and non-spec tests to match the relaxed
naming restrictions.
Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

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

Thanks for this! LGTM

@nbbeeken nbbeeken changed the title feat(NODE-3094): address dots-and-dollars changes feat!(NODE-3094): improve dots-and-dollars in keys experience Jun 28, 2021
@nbbeeken nbbeeken merged commit e376632 into mongodb:4.0 Jun 28, 2021
@addaleax addaleax deleted the 3094-dev branch June 28, 2021 14:13
ljhaywar pushed a commit that referenced this pull request Nov 9, 2021
Sets the BSON `checkKeys` option to false by default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants