8000 Fix ctx in tx begin by roman-mc · Pull Request #2260 · ent/ent · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix ctx in tx begin #2260

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Jan 12, 2022
Merged

Fix ctx in tx begin #2260

merged 7 commits into from
Jan 12, 2022

Conversation

roman-mc
Copy link
Contributor
@roman-mc roman-mc commented Jan 10, 2022 •
edited
Loading

This PR fixes #2253

  1. Fix old instruction in doc
  2. Fix missing context in TxBegin & Write test for it
  3. Run go generate in root folder

Tested 2-nd point according to docs, everything looks ok

@roman-mc roman-mc changed the title Fix ctx in tx begin Fix ctx in tx begin (#2253) Jan 10, 2022
@roman-mc roman-mc changed the title Fix ctx in tx begin (#2253) Fix ctx in tx begin Jan 10, 2022
a8m
a8m approved these changes Jan 11, 2022
View reviewed changes
Copy link
Member
@a8m a8m left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @Laconty.

Please, fix the minor comments and I'll merge this.

@roman-mc
Copy link
Contributor Author

Thanks for code review @a8m , I have one last question about this thread #2260 (comment)

@roman-mc roman-mc requested a review from a8m January 11, 2022 17:31
@roman-mc roman-mc force-pushed the fix_ctx_in_tx_begin branch from 79e3bac to 627ad9c Compare January 11, 2022 17:33
@roman-mc
Copy link
Contributor Author

Thanks @a8m and @masseelch for code-review and some clarifications, I think now it's ready

@a8m
Copy link
Member
a8m commented Jan 11, 2022

@Laconty, please run codegen (go generate ./...) and resubmit the PR. You may also want to add the email in the commit message to your GitHub account so GitHub will know to link this commit to your user after it's merged to master.

@roman-mc roman-mc force-pushed the fix_ctx_in_tx_begin branch from 627ad9c to a3026d9 Compare January 12, 2022 05:17
@roman-mc
Copy link
Contributor Author
roman-mc commented Jan 12, 2022 •
edited
Loading
  1. Run go generate in root folder
  2. re-author commits (thanks for your note)

Now I hope It's ready @a8m 🙂
(you wrote I should resubmit PR, I've changed author by some hustle, hope that's ok)

@roman-mc roman-mc force-pushed the fix_ctx_in_tx_begin branch from a3026d9 to 5e4907a Compare January 12, 2022 09:20
@a8m a8m merged commit 84070a0 into ent:master Jan 12, 2022
@a8m
Copy link
Member
a8m commented Jan 12, 2022

Welcome to the Ent community @Laconty, and really thanks for your contribution. It's appreciated!

@all-contributors please add @Laconty for code

@allcontributors allcontributors bot mentioned this pull request Jan 12, 2022
Merged
@allcontributors
Copy link
Contributor

@a8m

I've put up a pull request to add @Laconty! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@masseelch masseelch masseelch left review comments

@a8m a8m Awaiting requested review from a8m

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

client.BeginTx does not set transaction context
3 participants
@roman-mc @a8m @masseelch
0