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

GODRIVER-2323 Support int64 for 'n' field in insert, update, and delete ops. #905

Merged
merged 4 commits into from
Apr 11, 2022

Conversation

matthewdale
Copy link
Collaborator

GODRIVER-2323

Add support for int32 and int64 types in the "n" and "nModified" fields in various server CRUD responses.

Copy link
Contributor

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

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

Should we add a similar type switch in count.go to not error on the receiving of an int32 from the server?

@benjirewis
Copy link
Contributor

benjirewis commented Apr 8, 2022

There are also some lint failures for now unnecessary conversions to int64 in collection.go.

@matthewdale
Copy link
Collaborator Author

@benjirewis Added int32 support to count and fixed the linter failures.

Copy link
Contributor

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

Thank you for capturing the rationale in the ticket comments.

x/mongo/driver/operation/count.go Outdated Show resolved Hide resolved
x/mongo/driver/operation/delete.go Show resolved Hide resolved
Copy link
Contributor

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

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

LGTM after switching to AsInt64Ok(). Apologies for the misleading comment about count.go.

x/mongo/driver/operation/count.go Outdated Show resolved Hide resolved
Copy link
Contributor

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

LGTM!

@matthewdale matthewdale merged commit 5fe9dc5 into mongodb:master Apr 11, 2022
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.

4 participants