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

Change the method definition of UndoLogParser for better extensibility. #1090

Merged
merged 6 commits into from
May 28, 2019

Conversation

ujjboy
Copy link
Contributor

@ujjboy ujjboy commented May 22, 2019

Ⅰ. Describe what this PR did

  • change the method definition of UndoLogParser.
    • String encode(BranchUndoLog branchUndoLog) -> byte[] encode(BranchUndoLog branchUndoLog)
    • BranchUndoLog decode(String data) -> BranchUndoLog decode(byte[] bytes)
  • Use transient instead of @JSONField(serialize = false) for ignore serialization of field tableMeta
  • Refactor test case for more abstract.

Ⅱ. Does this pull request fix one issue?

Fix #1088

@codecov-io
Copy link

codecov-io commented May 22, 2019

Codecov Report

Merging #1090 into develop will decrease coverage by 0.13%.
The diff coverage is 24%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1090      +/-   ##
=============================================
- Coverage      41.87%   41.73%   -0.14%     
  Complexity      1361     1361              
=============================================
  Files            243      243              
  Lines          10112    10118       +6     
  Branches        1321     1325       +4     
=============================================
- Hits            4234     4223      -11     
- Misses          5329     5347      +18     
+ Partials         549      548       -1
Impacted Files Coverage Δ Complexity Δ
...o/seata/rm/datasource/sql/struct/TableRecords.java 73.68% <ø> (ø) 13 <0> (ø) ⬇️
...ata/rm/datasource/undo/JSONBasedUndoLogParser.java 100% <100%> (ø) 3 <2> (ø) ⬇️
...va/io/seata/rm/datasource/undo/UndoLogManager.java 22.64% <13.63%> (-0.59%) 8 <1> (ø)
...server/store/file/FileTransactionStoreManager.java 41.46% <0%> (-4.53%) 19% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0aeff8d...61d100e. Read the comment docs.

Copy link
Contributor

@zhangthen zhangthen left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@zhangthen zhangthen left a comment

Choose a reason for hiding this comment

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

Just modify the return type of "UndoLogParser", LGTM.

Copy link
Contributor

@leizhiyuan leizhiyuan left a comment

Choose a reason for hiding this comment

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

Please change the title ,It fixes a bug of JSON encode

@leizhiyuan
Copy link
Contributor

@github-ygy need your review.

Copy link
Member

@xingfudeshi xingfudeshi left a comment

Choose a reason for hiding this comment

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

LGTM.

@zhangthen zhangthen merged commit 71216f6 into apache:develop May 28, 2019
@ujjboy ujjboy deleted the change_param branch May 28, 2019 12:23
nick-tan pushed a commit to nick-tan/seata that referenced this pull request Jul 12, 2019
@wangliang181230 wangliang181230 added this to the 0.6.* milestone Aug 9, 2021
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.

Change the method definition of UndoLogParser for better extensibility.
6 participants