[Java] Breaking change for timeout parameters for SocketSettings builder methods #519
Conversation
rachel-mack
force-pushed
the
DOCSP-35763
branch
from
0dcc7c6 to
5409776
Compare
source/whats-new.txt
Outdated
| - The first parameter, time durations, of the following ``SocketSettings`` | ||
| builder methods is of ``long`` type: |
There was a problem hiding this comment.
Q: is the first parameter called "time durations"? If so I would monospace it (and I'm assuming camel-case it). I think being more specific with it will be more helpful.
Also I think we could be more concise here by just saying:
| - The first parameter, time durations, of the following ``SocketSettings`` | |
| builder methods is of ``long`` type: | |
| - The time durations parameter of the following ``SocketSettings`` | |
| builder methods is of ``long`` type: |
There was a problem hiding this comment.
The parameter names are connectTimeout and readTimeout, which is the first parameter of each. I've changed to your phrasing and changed it to "timeout", but have not monospaced it. I think it should be clear this way to anyone who's using either method.
source/upgrade.txt
Outdated
| guide. This change breaks binary compatibility (requires recompiling) but does not | ||
| require code changes. |
There was a problem hiding this comment.
Suggestion: I think we generally try to avoid putting statements in parenthesis like this if not necessary.
| guide. This change breaks binary compatibility (requires recompiling) but does not | |
| require code changes. | |
| guide. This change breaks binary compatibility and requires recompiling, but does not | |
| require code changes. |
| - The timeout parameters of the following ``SocketSettings`` | ||
| builder methods is of ``long`` type: |
There was a problem hiding this comment.
S: make parameters singular so it doesn't read like it's multiple for each one
| - The timeout parameters of the following ``SocketSettings`` | |
| builder methods is of ``long`` type: | |
| - The timeout parameter of the following ``SocketSettings`` | |
| builder methods is of ``long`` type: |
There was a problem hiding this comment.
The PR description states that this PR documents https://jira.mongodb.org/browse/DOCSP-35763, which is a duplicate of https://jira.mongodb.org/browse/DOCSP-34994 (I wonder, why it was created). And the latter has already been reviewed as part pf #507.
Moreover, I see many changes unrelated to https://jira.mongodb.org/browse/DOCSP-35763 in this PR. Why are they here?
I'll wait until this PR gets well-organized/tidied up before proceeding with reviewing it. However, as far as I can see, all the changes here must have been already documented in other PRs, and this PR should probably be just closed.
You're right - this is a duplicate and I'll remove it. |
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-35763
Staging
This includes a few weird changes that are artifacts from a rebase.
Self-Review Checklist