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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify and add tests for GUID changing #27

Merged
merged 3 commits into from
Feb 25, 2019
Merged

Conversation

linabutler
Copy link
Member

As promised. 馃槃 This also takes the good parts of #26 (checking for dupes, changing GUIDs for nodes on both sides, and comments for what we reupload and why).

@codecov-io
Copy link

codecov-io commented Feb 23, 2019

Codecov Report

Merging #27 into master will increase coverage by 0.6%.
The diff coverage is 94%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #27     +/-   ##
=========================================
+ Coverage   91.51%   92.11%   +0.6%     
=========================================
  Files           7        7             
  Lines        2805     2855     +50     
=========================================
+ Hits         2567     2630     +63     
+ Misses        238      225     -13
Impacted Files Coverage 螖
src/tests.rs 99.9% <100%> (-0.01%) 猬囷笍
src/guid.rs 84.03% <88.46%> (+0.36%) 猬嗭笍
src/merge.rs 93.61% <93.75%> (+0.37%) 猬嗭笍
src/tree.rs 82.09% <0%> (+1.93%) 猬嗭笍
src/error.rs 47.36% <0%> (+9.86%) 猬嗭笍
src/lib.rs 100% <0%> (+100%) 猬嗭笍

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 8dc54ef...4c81c81. Read the comment docs.

* If the merged parent's GUID changed, we need to flag its children for
  reupload, so that the childrens' `parentid` is correct.
* If the merged child's GUID changed, we need to flag its parent for
  reupload, so that the parent's `children` are correct.
* It's an error for `Driver::generate_new_guid` to return a GUID that we
  already merged.
Copy link
Contributor

@thomcc thomcc left a comment

Choose a reason for hiding this comment

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

Love the rename!

@linabutler linabutler merged commit 4c81c81 into master Feb 25, 2019
@linabutler linabutler deleted the cleanup-guid-changes branch February 25, 2019 22:01
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.

None yet

3 participants