-
Notifications
You must be signed in to change notification settings - Fork 902
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-3499 Update v2 migration guide. #1987
Conversation
API Change ReportNo changes found! |
docs/migration-2.0.md
Outdated
### UpdateManyOptions / UpdateOneOptions | ||
|
||
The `UpdateOptions` has been separated into `UpdateManyOptions` and `UpdateOneOptions` to configure the corresponding `UpdateMany` and `UpdateOne` operations. | ||
|
||
The data type of `ArrayFilters` in the `Update*Options` has been changed to `[]interface{}`, which can be used as the `Filters` field in the original `ArrayFilters` struct. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the * in Update*Options
? is this meant to be regex? i suggest we simply write out "UpdateManyOptions
and UpdateOneOptions
" to avoid extra confusion.
also is there a word missing after "the Update*Options
"? should it be "the Update*Options
types"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UpdateManyOptions
and UpdateOneOptions
in v2 are derived from the UpdateOptions
in v1, so the ArrayFilters
field in both types is also adopted from UpdateOptions
. I'm not sure if using "UpdateManyOptions
and UpdateOneOptions
" is clear enough in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand. What does Update*Options
refer to in your statement above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think the following is clearer?
The data type of `ArrayFilters` in the `Update*Options` has been changed to `[]interface{}`, which can be used as the `Filters` field in the original `ArrayFilters` struct. | |
The `ArrayFilters` struct type has been removed in v2. Correspondingly, the data type of `ArrayFilters` in the `UpdateOptions` struct has been changed to `[]interface{}` in the `UpdateManyOptions` and `UpdateOneOptions` structs. The new `ArrayFilters` field (with the `[]interface{}` type) can be used as the `Filters` field in the original `ArrayFilters` struct. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked up the code change and see what you mean now. What made this confusing is that both the field and the struct type had the same name.
How about this:
The data type of `ArrayFilters` in the `Update*Options` has been changed to `[]interface{}`, which can be used as the `Filters` field in the original `ArrayFilters` struct. | |
The `ArrayFilters` struct type has been removed in v2. As a result, the `ArrayFilters` fields in the new `UpdateManyOptions` and `UpdateOneOptions` structs (which replace the old `UpdateOptions` struct) now use the `[]interface{}` type. The `ArrayFilters` field (now of type `[]interface{}`) serves the same purpose as the `Filters` field in the original `ArrayFilters` struct. |
docs/migration-2.0.md
Outdated
@@ -633,10 +633,14 @@ The following types are not valid for a `findOne` operation and have been remove | |||
- `MaxAwaitTime` | |||
- `NoCursorTimeout` | |||
|
|||
The data type of `ArrayFilters` has been changed to `[]interface{}`, which can be used as the `Filters` field in the original `ArrayFilters` struct. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would "and" possibly make more sense that "which" here? will defer to @matthewdale
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one should be similar to the one below right?
The data type of `ArrayFilters` has been changed to `[]interface{}`, which can be used as the `Filters` field in the original `ArrayFilters` struct. | |
The `ArrayFilters` struct type has been removed in v2. As a result, the `ArrayFilters` field in the `FindOneAndUpdateOptions` struct now uses the `[]interface{}` type. The `ArrayFilters` field (now of type `[]interface{}`) serves the same purpose as the `Filters` field in the original `ArrayFilters` struct. |
Should we change this section to "### FindOneOptions / FindOneAndUpdateOptions"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - thanks, @qingyang-hu!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 👍
GODRIVER-3499
Summary
Update the v2 migration guide on
ClientOptions.AuthenticateToAnything
andArrayFilters
.Background & Motivation