-
Notifications
You must be signed in to change notification settings - Fork 43
DOCSP-35011: ConnectionId class changes #495
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
DOCSP-35011: ConnectionId class changes #495
Conversation
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.
Just a few suggestions/style guide fixes
source/upgrade.txt
Outdated
|
||
The ``ConnectionId`` constructor now accepts a value of type ``long`` as its second | ||
parameter instead of type ``int``. Similarly, the constructor now accepts a value of | ||
type ``Long`` as its third parameter instead of type ``Integer``. Since this change breaks |
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.
Style Guide: Don't use "since" to explain why, instead use "because"
type ``Long`` as its third parameter instead of type ``Integer``. Since this change breaks | |
type ``Long`` as its third parameter instead of type ``Integer``. Because this change breaks |
source/upgrade.txt
Outdated
binary compatibility, recompile any existing code that calls the ``ConnectionId`` constructor. | ||
|
||
The ``withServerValue()`` method now accepts a parameter of type ``long`` rather than | ||
type ``int``. This change also breaks binary compatibility, so you must recompile any code |
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.
Suggestion: I don't think "also" is needed here, especially if a reader didn't read the previous change
type ``int``. This change also breaks binary compatibility, so you must recompile any code | |
type ``int``. This change breaks binary compatibility, so you must recompile any code |
source/upgrade.txt
Outdated
|
||
The ``getServerValue()`` method now returns a value of type ``Long`` instead of type | ||
``Integer``. Similarly, the ``getLocalValue()`` method returns a value of type | ||
``long`` instead of type ``int``. Since this change breaks both binary and source |
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.
Same as above comment regarding "since"
``long`` instead of type ``int``. Since this change breaks both binary and source | |
``long`` instead of type ``int``. Because this change breaks both binary and source |
source/upgrade.txt
Outdated
|
||
- Several changes were made to the ``ConnectionId`` class. | ||
|
||
The ``ConnectionId`` constructor now accepts a value of type ``long`` as its second |
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 think these could be nested bullets for easier readability (applies to all nested paragraphs in this section)
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!
This is a Java 5.0 ticket.
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-35011
Staging:
https://preview-mongodbnorareidy.gatsbyjs.io/java/DOCSP-35011-connectionid-return-types/whats-new/
https://preview-mongodbnorareidy.gatsbyjs.io/java/DOCSP-35011-connectionid-return-types/upgrade/#std-label-java-breaking-changes-v5.0
Self-Review Checklist