Skip to content
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-41760 Add transactions page #167

Merged

Conversation

stephmarie17
Copy link
Collaborator

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-41760
Staging - https://preview-mongodbstephmarie17.gatsbyjs.io/kotlin/docsp-41760-addTransactionsPage/fundamentals/transactions/

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
Collaborator

@mcmorisi mcmorisi left a comment

Choose a reason for hiding this comment

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

The copy looks good, just some notes on the code sample!

@stephmarie17 stephmarie17 requested a review from mcmorisi July 31, 2024 18:17
Copy link
Collaborator

@mcmorisi mcmorisi left a comment

Choose a reason for hiding this comment

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

LGTM with one last suggestion!

source/fundamentals/transactions.txt Outdated Show resolved Hide resolved
Copy link

netlify bot commented Aug 20, 2024

Deploy Preview for docs-kotlin ready!

Name Link
🔨 Latest commit 2d7c437
🔍 Latest deploy log https://app.netlify.com/sites/docs-kotlin/deploys/66ccdfce01b1460008a686eb
😎 Deploy Preview https://deploy-preview-167--docs-kotlin.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@stephmarie17
Copy link
Collaborator Author

Tagging @mongodb/dbx-java for technical review 🙇‍♀️

@stIncMale stIncMale requested a review from jyemin August 26, 2024 18:47
@jyemin
Copy link
Collaborator

jyemin commented Aug 27, 2024

Am I correct that this page doesn't exist yet for the Java sync driver? I was looking to compare the two.

@stephmarie17
Copy link
Collaborator Author

stephmarie17 commented Aug 27, 2024

Am I correct that this page doesn't exist yet for the Java sync driver? I was looking to compare the two.

@jyemin Correct, we're in the process of adding these pages. There are a few PRs for Java-related transactions pages in progress:

Then one for Kotin sync will be upcoming. There are some examples of existing Transaction pages for other drivers in the JIRA ticket if that is helpful!

Copy link
Collaborator

@jyemin jyemin left a comment

Choose a reason for hiding this comment

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

Everything looks good but wondering whether we should document com.mongodb.kotlin.client.ClientSession#withTransaction here as well, or if that's going in a different PR.

@stephmarie17
Copy link
Collaborator Author

@jyemin thanks for taking a look! I didn't see withTransaction in the API docs so I wasn't sure if that was supported. Is that supported and is there more info someplace in the API docs that I missed?

One other question I have: once this PR is approved and published, to what version of Kotlin Coroutine should we backport the docs update? The docs versions are upcoming, 5.1, 5.0, 4.11, and 4.10.

@jyemin
Copy link
Collaborator

jyemin commented Aug 28, 2024

I didn't see withTransaction in the API docs so I wasn't sure if that was supported. Is that supported and is there more info someplace in the API docs that I missed?

Oh, I thought this was a Kotlin Sync Driver page. Are you sure we don't need some sort of call to runBlocking anywhere?

One other question I have: once this PR is approved and published, to what version of Kotlin Coroutine should we backport the docs update? The docs versions are upcoming, 5.1, 5.0, 4.11, and 4.10.

It's up to docs team how far all these pages will be backported.

@stephmarie17
Copy link
Collaborator Author

I didn't see withTransaction in the API docs so I wasn't sure if that was supported. Is that supported and is there more info someplace in the API docs that I missed?

Oh, I thought this was a Kotlin Sync Driver page. Are you sure we don't need some sort of call to runBlocking anywhere?

Will be working on the Kotlin Sync page soon! There's a call to runBlocking in the example file (examples/src/test/kotlin/TransactionsTest.kt) but the snippet displayed in the docs starts after that.

@jyemin jyemin self-requested a review August 28, 2024 18:49
Copy link
Collaborator

@jyemin jyemin left a comment

Choose a reason for hiding this comment

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

LGTM

@stephmarie17 stephmarie17 merged commit e0c22ee into mongodb:master Aug 29, 2024
8 of 9 checks passed
@docs-builder-bot
Copy link
Collaborator

rustagir pushed a commit that referenced this pull request Aug 29, 2024
* Add transactions page, add language variable, include code snippet for transactions

---------

Co-authored-by: Rea Rustagi <[email protected]>
Co-authored-by: Brandon Ly <[email protected]>
Co-authored-by: rustagir <[email protected]>
(cherry picked from commit e0c22ee)
stephmarie17 added a commit to stephmarie17/docs-kotlin that referenced this pull request Aug 29, 2024
* Add transactions page, add language variable, include code snippet for transactions

---------

Co-authored-by: Rea Rustagi <[email protected]>
Co-authored-by: Brandon Ly <[email protected]>
Co-authored-by: rustagir <[email protected]>
(cherry picked from commit e0c22ee)
stephmarie17 added a commit that referenced this pull request Aug 29, 2024
* Add transactions page, add language variable, include code snippet for transactions

---------

Co-authored-by: Rea Rustagi <[email protected]>
Co-authored-by: Brandon Ly <[email protected]>
Co-authored-by: rustagir <[email protected]>
(cherry picked from commit e0c22ee)
stephmarie17 added a commit that referenced this pull request Aug 29, 2024
* Add transactions page, add language variable, include code snippet for transactions

---------

Co-authored-by: Rea Rustagi <[email protected]>
Co-authored-by: Brandon Ly <[email protected]>
Co-authored-by: rustagir <[email protected]>
(cherry picked from commit e0c22ee)
stephmarie17 added a commit that referenced this pull request Aug 29, 2024
* Add transactions page, add language variable, include code snippet for transactions

---------

Co-authored-by: Rea Rustagi <[email protected]>
Co-authored-by: Brandon Ly <[email protected]>
Co-authored-by: rustagir <[email protected]>
(cherry picked from commit e0c22ee)
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.

6 participants