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

[improve][doc]Add introduction of transaction isolation level in the txn-advanced-features.md #712

Merged
merged 15 commits into from
Feb 29, 2024

Conversation

hzh0425
Copy link
Member

@hzh0425 hzh0425 commented Oct 6, 2023

This PR adds doc for apache/pulsar#21114

This PR fixes #xyz

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@github-actions github-actions bot added the doc Improvements or additions to documentation label Oct 6, 2023
@tisonkun
Copy link
Member

We should not merge this patch until PIP-298 is accepted and merged. Is it correct?

docs/txn-advanced-features.md Outdated Show resolved Hide resolved
docs/txn-advanced-features.md Outdated Show resolved Hide resolved
@visortelle
Copy link
Member

The PIP-298 has been merged: apache/pulsar#21114

I applied slight changes that shouldn't affect sense much:
384084d...2b4c747#diff-eefcce381353ec170e6087b3c59f413c4e3f6081f93de753dd2714d725bebdbd

Merging.

@visortelle visortelle merged commit b7ada2f into apache:main Feb 29, 2024
2 checks passed
@shibd
Copy link
Member

shibd commented May 27, 2024

@visortelle Please make sure someone approves and all concerns are addressed before merging the PR.

@tisonkun
Copy link
Member

@shibd Who is "someone"? I'd prefer you name the list of component experts so that @visortelle can ping for review.

Note that @visortelle is a committer and we don't have class among committers but if you know experts that should review changes on these components, we can collaborate.

@shibd
Copy link
Member

shibd commented May 27, 2024

I just want to remind @visortelle that PRs should be reviewed by someone before they can be merged, and we should also take comments into consideration.

@visortelle
Copy link
Member

@shibd ok, no problem. Let me know if something is wrong with this specific PR.

@shibd
Copy link
Member

shibd commented May 27, 2024

Thanks, This PR should not be merged because it has not been implemented yet. Please refer to this comment: #712 (comment)

@visortelle
Copy link
Member

I agree, it is my fault.

I think it makes sense to:

  • create a PR with the revert commit
  • create a new draft PR with the same changes

@shibd WDYT?

visortelle added a commit that referenced this pull request May 27, 2024
… in the txn-advanced-features.md (#712)"

This reverts commit b7ada2f.
visortelle added a commit that referenced this pull request May 27, 2024
… in the txn-advanced-features.md (#712)" (#904)

This reverts commit b7ada2f.
visortelle added a commit that referenced this pull request May 27, 2024
…on level in the txn-advanced-features.md (#712)""

This reverts commit 6ff9b3e.
visortelle added a commit that referenced this pull request May 27, 2024
…txn-advanced-features.md (#712)

* Add introduction of transaction isolation level in the txn-advanced-features.md

Co-authored-by: Kiryl Valkovich <[email protected]>
@visortelle
Copy link
Member

@shibd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants