Skip to content

5.0 Removals - Rebase #521

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Feb 9, 2024
Merged

5.0 Removals - Rebase #521

merged 10 commits into from
Feb 9, 2024

Conversation

rachel-mack
Copy link
Contributor

@rachel-mack rachel-mack commented Feb 7, 2024 •
edited
Loading

@rachel-mack rachel-mack marked this pull request as ready for review February 8, 2024 14:06
@rachel-mack rachel-mack requested a review from stIncMale February 8, 2024 14:06
@stIncMale
Copy link
Member

I don't find dismissing some of the work done in #510 and re-reviewing source/upgrade.txt to be an efficient approach. If the changes cannot continue in #510 for some reason (I don't know what that reason is), I would expect this PR to have changes identical to those done in #510, except for the part described in #510 (comment).

Currently, the above is not the case, and the new (current) PR, has duplicate description of the changes done to ConnectionId. There are other discrepancies. Let's make this PR consistent with the changes made in #510 in all places, where possible, and then move forward from there.

stIncMale
stIncMale requested changes Feb 8, 2024
View reviewed changes
Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

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

<A useless comment foolishly required by GitHub to allow leaving a "Request changes" review.>

rachel-mack
rachel-mack commented Feb 8, 2024
View reviewed changes
@rachel-mack rachel-mack requested a review from stIncMale February 8, 2024 21:39
stIncMale
stIncMale requested changes Feb 9, 2024
View reviewed changes
Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

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

.

rachel-mack
rachel-mack commented Feb 9, 2024
View reviewed changes
source/upgrade.txt
@@ -61,8 +61,10 @@ the version after v4.0 including any listed under v4.5.
Version 5.0 Breaking Changes
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

- This driver version introduces the following changes to the ``ConnectionId`` class:
This driver version introduces the following breaking changes:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The introduction of this phrasing required minor changes to the beginning of several bullet points.

rachel-mack
rachel-mack commented Feb 9, 2024
View reviewed changes
source/upgrade.txt
Comment on lines +90 to +99
- Changes the data type of the timeout duration parameter in the
``SocketSettings.Builder.connectTimeout()`` and
``SocketSettings.Builder.readTimeout()`` methods. The data type of this
parameter is now ``long`` instead of ``int``.

In earlier versions, this parameter is of type ``int`` for both methods. This
change breaks binary compatibility and requires recompiling, but does not
require code changes. To view an example that shows how to call
``SocketSettings`` methods, see the :ref:`SocketSettings Example
<java-socketsettings-example>` in the Specify MongoClient Settings guide.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While this appears at first glance to be a major change, it's actually a minor rewrite to accommodate the new phrasing.

@rachel-mack rachel-mack requested a review from stIncMale February 9, 2024 08:35
@rachel-mack rachel-mack merged commit c091148 into mongodb:master Feb 9, 2024
@stIncMale stIncMale mentioned this pull request Feb 9, 2024
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stIncMale stIncMale stIncMale 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
@rachel-mack @stIncMale @mongoKart