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.

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

*: remove fail field from TxnState (#8712) #8764

Merged
merged 6 commits into from
Dec 25, 2018
Merged

*: remove fail field from TxnState (#8712) #8764

merged 6 commits into from
Dec 25, 2018

Conversation

tiancaiamao
Copy link
Contributor

@tiancaiamao tiancaiamao commented Dec 21, 2018 •
edited by c4pt0r
Loading

cherry pick from #8712

Please review session.go carefully.

@lysu @winkyao @jackysp

This change is Reviewable

@tiancaiamao
*: remove fail field from TxnState (pingcap#8712)
eba1d40 
Originally, fail is added to simplify error handling, avoiding large changes on
the Txn() interface API. But it turns out that fail introduces complexity.
So this commit removes the fail and makes an API refactoring.
@tiancaiamao
Copy link
Contributor Author

/run-all-tests tidb-test=pr/708

@tiancaiamao
Copy link
Contributor Author

/run-all-tests tidb-test=pr/708

1 similar comment
@tiancaiamao
Copy link
Contributor Author

/run-all-tests tidb-test=pr/708

@tiancaiamao
Copy link
Contributor Author

/run-all-tests tidb-test=pr/708

@tiancaiamao
Copy link
Contributor Author

/rebuild

@tiancaiamao
Copy link
Contributor Author

/run-all-tests tidb-test=pr/708

@tiancaiamao
Copy link
Contributor Author

@winkyao

zimulala
zimulala reviewed Dec 25, 2018
View reviewed changes
Copy link
Contributor

@zimulala zimulala left a comment

Choose a reason for hiding this comment

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

LGTM

@zimulala zimulala added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 25, 2018
jackysp
jackysp reviewed Dec 25, 2018
View reviewed changes
Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

lysu
lysu approved these changes Dec 25, 2018
View reviewed changes
Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

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

LGTM

@lysu lysu added require-LGT3 Indicates that the PR requires three LGTM. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 25, 2018
@tiancaiamao tiancaiamao added the status/LGT3 The PR has already had 3 LGTM. label Dec 25, 2018
@crazycs520
Copy link
Contributor

/run-all-tests tidb-test=release-2.0 tikv=release-2.0 pd=release-2.0

@jackysp
Copy link
Member

jackysp commented Dec 25, 2018

/run-integration-ddl-test tidb-test=pr/708 tikv=release-2.0 pd=release-2.0

@jackysp jackysp merged commit 305a083 into pingcap:release-2.0 Dec 25, 2018
@tiancaiamao tiancaiamao deleted the cherry-2.0 branch December 25, 2018 10:41
tiancaiamao added a commit that referenced this pull request Jan 2, 2019
@tiancaiamao
Revert "*: remove fail field from TxnState (#8712) (#8764)"
ac43620 
This reverts commit 305a083.
@tiancaiamao tiancaiamao mentioned this pull request Jan 2, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zimulala zimulala zimulala left review comments

@jackysp jackysp jackysp left review comments

@lysu lysu lysu approved these changes

Assignees
No one assigned
Labels
require-LGT3 Indicates that the PR requires three LGTM. sig/transaction SIG:Transaction status/LGT3 The PR has already had 3 LGTM.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tiancaiamao @crazycs520 @jackysp @lysu @zimulala