Skip to content

Conversation

rustagir
Copy link
Contributor

@rustagir rustagir commented Aug 2, 2024

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-41819
Staging - https://preview-mongodbrustagir.gatsbyjs.io/kotlin-sync/DOCSP-41819-replace/write/replace/

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

netlify bot commented Aug 2, 2024

👷 Deploy request for docs-kotlin-sync pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit fdb322c

Copy link
Collaborator

@shuangela shuangela left a comment

Choose a reason for hiding this comment

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

looks good, one confusion about the code

operations differs from an update operation, which changes only
specified fields in one or more documents.

.. tip:: Learn About Update Operations
Copy link
Collaborator

Choose a reason for hiding this comment

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

S: Make this part of the text rather than a callout, I'm unsure if making this is a callout is necessary


The values of ``_id`` fields are immutable. If your replacement document specifies
a value for the ``_id`` field, it must be the same as the ``_id`` value of the
existing document.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q: If the id value is not the same as the existing document, what happens? An error?

- Description

* - ``getMatchedCount()``
- | Indicates the number of documents that matched the query filter, regardless of
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- | Indicates the number of documents that matched the query filter, regardless of
- | Returns the number of documents that matched the query filter, regardless of

how many updates were performed.

* - ``getModifiedCount()``
- | Indicates number of documents modified by the update operation. If an updated
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- | Indicates number of documents modified by the update operation. If an updated
- | Returns number of documents modified by the update operation. If an updated

@rustagir rustagir requested a review from shuangela August 2, 2024 15:24
Copy link
Collaborator

@shuangela shuangela left a comment

Choose a reason for hiding this comment

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

lgtm!

@rustagir rustagir requested a review from rozza August 2, 2024 15:26
@@ -0,0 +1,41 @@
import com.mongodb.client.model.Filters
import com.mongodb.client.model.Filters.*
Copy link
Member

Choose a reason for hiding this comment

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

Nit - is this used?

Copy link
Member

@rozza rozza 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 of nits / questions about the Filters.* and Updates.* imports. I dont think they are needed.

import com.mongodb.client.model.Filters.*
import com.mongodb.client.model.ReplaceOptions
import com.mongodb.client.model.UpdateOptions
import com.mongodb.client.model.Updates.*
Copy link
Member

Choose a reason for hiding this comment

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

Nit - is this used?

@rustagir rustagir merged commit ad61726 into mongodb:master Aug 7, 2024
1 of 2 checks passed
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.

3 participants