Skip to content

[JAVA] Relax the CMAP documentation requirement for durations in events #501

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
merged 3 commits into from
Jan 18, 2024

Conversation

rachel-mack
Copy link
Contributor

@rachel-mack rachel-mack commented Jan 15, 2024

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-35270
Staging - https://docs-mongodbcom-staging.corp.mongodb.com/java/docsworker-xlarge/DOCSP-35270/whats-new/#std-label-version-5.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?

@rachel-mack rachel-mack marked this pull request as ready for review January 16, 2024 16:20
Comment on lines 57 to 73
- The ``ConnectionCheckedOutEvent.getElapsedTime`` method now includes the time
taken to create the connection. That is, the "timer" starts before the
connection is confirmed, rather than after.

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 more technical detail would be helpful here. I'd probably just paste in the Jira notes and maybe edit them slightly.

Also, I think the ticket describes more changes than I see here; is there another ticket for those?

@rachel-mack
Copy link
Contributor Author

Note: Vale error is for use of "Fail" in the event name.

@rachel-mack rachel-mack requested a review from mongoKart January 17, 2024 16:25
Copy link
Contributor

@mongoKart mongoKart 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 couple quick changes!

@@ -59,6 +59,18 @@ The 5.0 driver release introduces the following features:
``isNumber()`` and ``asNumber()`` methods match the Java responses for
equivalent ``Decimal128`` values.

- The ``getElapsedTime`` method on ``com.mongodb.event.ConnectionReadyEvent``
includes the time taken to deliver the ``ConnectionPoolListener``. That is,
Copy link
Contributor

Choose a reason for hiding this comment

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

I: per ticket, i think this should be:

Suggested change
includes the time taken to deliver the ``ConnectionPoolListener``. That is,
includes the time taken to deliver the ``ConnectionCreatedEvent``. That is,

@@ -59,6 +59,18 @@ The 5.0 driver release introduces the following features:
``isNumber()`` and ``asNumber()`` methods match the Java responses for
equivalent ``Decimal128`` values.

- The ``getElapsedTime`` method on ``com.mongodb.event.ConnectionReadyEvent``
Copy link
Contributor

Choose a reason for hiding this comment

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

I: In all of our docs that I've seen, we suffix method names with parentheses

Suggested change
- The ``getElapsedTime`` method on ``com.mongodb.event.ConnectionReadyEvent``
- The ``getElapsedTime()`` method on ``com.mongodb.event.ConnectionReadyEvent``

Comment on lines 64 to 65
the time returned includes the duration of
``com.mongodb.event.ConnectionPoolListener.connectionCreated``.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
the time returned includes the duration of
``com.mongodb.event.ConnectionPoolListener.connectionCreated``.
the time returned includes the duration of the
``com.mongodb.event.ConnectionPoolListener.connectionCreated()`` method.

the time returned includes the duration of
``com.mongodb.event.ConnectionPoolListener.connectionCreated``.

The ``getElapsedTime`` methods on
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The ``getElapsedTime`` methods on
The ``getElapsedTime()`` methods on

Comment on lines 71 to 72
time returned includes the duration of
``com.mongodb.eventConnectionPoolListener.connectionCheckOutStarted``.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
time returned includes the duration of
``com.mongodb.eventConnectionPoolListener.connectionCheckOutStarted``.
time returned includes the duration of the
``com.mongodb.eventConnectionPoolListener.connectionCheckOutStarted()`` method.

@rachel-mack rachel-mack requested a review from mongoKart January 18, 2024 17:09
Copy link
Contributor

@mongoKart mongoKart left a comment

Choose a reason for hiding this comment

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

LGTM

@rachel-mack rachel-mack merged commit fd9690f into mongodb:master Jan 18, 2024
@rachel-mack rachel-mack deleted the DOCSP-35270 branch January 18, 2024 19:49
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