Skip to content

GODRIVER-2976 Remove deprecated BSON code (phase 1)#1410

Merged
qingyang-hu merged 14 commits into
mongodb:masterfrom
qingyang-hu:godriver2976
Jan 10, 2024
Merged

GODRIVER-2976 Remove deprecated BSON code (phase 1)#1410
qingyang-hu merged 14 commits into
mongodb:masterfrom
qingyang-hu:godriver2976

Conversation

@qingyang-hu

@qingyang-hu qingyang-hu commented Oct 4, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

GODRIVER-2976

Summary

Phase 1 of the deprecation to limit the files to reviewing one time.

Background & Motivation

@mongodb-drivers-pr-bot

mongodb-drivers-pr-bot Bot commented Nov 29, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

API Change Report

./bson

incompatible changes

##(*Decoder).Reset: changed from func(./bson/bsonrw.ValueReader) error to func(./bson/bsonrw.ValueReader)
(Decoder).SetContext: removed
##(Decoder).SetRegistry: changed from func(./bson/bsoncodec.Registry) error to func(./bson/bsoncodec.Registry)
##(*Encoder).Reset: changed from func(./bson/bsonrw.ValueWriter) error to func(./bson/bsonrw.ValueWriter)
(Encoder).SetContext: removed
##(Encoder).SetRegistry: changed from func(./bson/bsoncodec.Registry) error to func(./bson/bsoncodec.Registry)
MarshalAppend: removed
MarshalAppendWithContext: removed
MarshalAppendWithRegistry: removed
MarshalExtJSONAppend: removed
MarshalExtJSONAppendWithContext: removed
MarshalExtJSONAppendWithRegistry: removed
MarshalExtJSONWithContext: removed
MarshalExtJSONWithRegistry: removed
MarshalValueAppend: removed
MarshalValueAppendWithContext: removed
MarshalValueAppendWithRegistry: removed
MarshalValueWithContext: removed
MarshalWithContext: removed
MarshalWithRegistry: removed
NewDecoderWithContext: removed
##NewEncoder: changed from func(./bson/bsonrw.ValueWriter) (*Encoder, error) to func(./bson/bsonrw.ValueWriter) *Encoder
NewEncoderWithContext: removed
NewFromIOReader: removed
RawValue.AsInt32: removed
RawValue.AsInt32OK: removed

@qingyang-hu qingyang-hu force-pushed the godriver2976 branch 3 times, most recently from 999bdd8 to 6409678 Compare December 5, 2023 18:14
@qingyang-hu qingyang-hu force-pushed the godriver2976 branch 2 times, most recently from 3c6dfdb to 6cc5e96 Compare December 6, 2023 19:13
@qingyang-hu qingyang-hu force-pushed the godriver2976 branch 3 times, most recently from 720f32b to 50c8cf1 Compare December 7, 2023 23:08
@qingyang-hu qingyang-hu changed the title GODRIVER-2976 Remove deprecated BSON code GODRIVER-2976 Remove deprecated BSON code (phase 1) Dec 8, 2023
@qingyang-hu qingyang-hu marked this pull request as ready for review December 8, 2023 15:55
@qingyang-hu qingyang-hu requested a review from a team as a code owner December 8, 2023 15:55
@qingyang-hu qingyang-hu requested review from blink1073 and removed request for a team December 8, 2023 15:55
@qingyang-hu qingyang-hu requested review from matthewdale and prestonvasquez and removed request for blink1073 December 8, 2023 15:56
matthewdale
matthewdale reviewed Dec 20, 2023
View reviewed changes
Comment thread bson/bson_test.go Outdated
Comment on lines +181 to +182
buf := new(bytes.Buffer)
enc := new(Encoder)

@matthewdale matthewdale Dec 20, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's technically safe to share the buffer and Encoder in this case, but it creates an opportunity for unexpected interaction between test cases. We should initialize these values in the test case funcs.

Comment thread bson/marshal_test.go Outdated
Comment on lines +30 to +31
buf := new(bytes.Buffer)
enc := new(Encoder)

@matthewdale matthewdale Dec 20, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should initialize these values in the test case funcs.

Comment thread bson/marshal_test.go Outdated
Comment on lines +57 to +58
buf := new(bytes.Buffer)
enc := new(Encoder)

@matthewdale matthewdale Dec 20, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should initialize these values in the test case funcs.

Comment thread mongo/options/mongooptions.go
Comment thread mongo/options/mongooptions.go
Comment thread internal/integration/unified/gridfs_bucket_operation_execution.go Outdated
filename = val.StringValue()
case "revision":
opts.SetRevision(val.AsInt32())
opts.SetRevision(int32(val.AsInt64()))

@matthewdale matthewdale Dec 20, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Check this conversion to make sure it doesn't overflow.

prestonvasquez
prestonvasquez previously approved these changes Jan 3, 2024
View reviewed changes

@prestonvasquez prestonvasquez left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, with changes requested by @matthewdale 👍

matthewdale
matthewdale approved these changes Jan 10, 2024
View reviewed changes

@matthewdale matthewdale left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! 👍

@qingyang-hu qingyang-hu merged commit 8c5c0b0 into mongodb:master Jan 10, 2024
@prestonvasquez prestonvasquez mentioned this pull request Apr 16, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@matthewdale matthewdale matthewdale approved these changes

@prestonvasquez prestonvasquez prestonvasquez approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@qingyang-hu @matthewdale @prestonvasquez