Skip to content

Fix empty metadata after message cloning #247

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 1 commit into from
Apr 13, 2019
Merged

Fix empty metadata after message cloning #247

merged 1 commit into from
Apr 13, 2019

Conversation

msiebeneicher
Copy link
Contributor

The purpose of this pull request

  • fixed cloning metadata
  • fixed unit test for cloning metadata

Checklist

  • make test executed successfully
  • unit test provided

- fixed cloning metadata
f6bc639 
- fixed unit test for cloning metadata
@msiebeneicher msiebeneicher requested a review from arnecls April 12, 2019 15:31
@msiebeneicher msiebeneicher self-assigned this Apr 12, 2019
@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 71.652% when pulling f6bc639 on bugfix/fix_empty_meta_data_after_cloning into 892ba2d on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 71.652% when pulling f6bc639 on bugfix/fix_empty_meta_data_after_cloning into 892ba2d on master.

@arnecls arnecls merged commit 3b6429b into master Apr 13, 2019
@arnecls arnecls deleted the bugfix/fix_empty_meta_data_after_cloning branch April 13, 2019 20:17
arnecls added a commit that referenced this pull request Apr 13, 2019
@arnecls
cherry pick PRs #246 amd #247
4ce38f3
@arnecls
Copy link
Contributor

arnecls commented Apr 13, 2019

Found and fixed an issue after merging.
The thing with CloneOriginal is that we create a copy of the current state first and then either need to clone the current metadata (to avoid changes "leaking" into the copy) or explicitly set it to nil as the original (as defined by the "freeze" operation) does not have metadata.

See commit: 7c14a24

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

@dsiebel dsiebel dsiebel approved these changes

@arnecls arnecls arnecls approved these changes

Assignees

@msiebeneicher msiebeneicher

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@msiebeneicher @coveralls @arnecls @dsiebel