Skip to content

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

Merged

Conversation

norareidy
Copy link
Contributor

@norareidy norareidy commented Jan 10, 2024

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

  • Is this free of any warnings or errors in the RST?
  • Did you run a spell-check?
  • Did you run a grammar-check?
  • Are all the links working?
  • Are the facets and meta keywords accurate?

Copy link
Contributor

@jordan-smith721 jordan-smith721 left a 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


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
Copy link
Contributor

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"

Suggested change
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

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
Copy link
Contributor

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

Suggested 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


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
Copy link
Contributor

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"

Suggested change
``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


- Several changes were made to the ``ConnectionId`` class.

The ``ConnectionId`` constructor now accepts a value of type ``long`` as its second
Copy link
Contributor

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)

Copy link
Contributor

@jordan-smith721 jordan-smith721 left a comment

Choose a reason for hiding this comment

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

LGTM!

@norareidy norareidy merged commit 7bb84d7 into mongodb:master Jan 11, 2024
@norareidy norareidy deleted the DOCSP-35011-connectionid-return-types branch January 11, 2024 21:56
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.

2 participants