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

Allow merge regions to cross pages #158

Merged
merged 7 commits into from
Oct 19, 2021
Merged

Conversation

Shillaker
Copy link
Collaborator

@Shillaker Shillaker commented Oct 18, 2021

Previously merge regions could only be up to one page long. This is inconvenient and non-obvious to users of the API.

Changes:

  • Allow merge regions to extend over multiple pages.
  • Add tests and bug fixes around edge cases that weren't previously caught.
  • Add trace logging to diffing algorithm to aid debugging.

@Shillaker Shillaker marked this pull request as draft October 18, 2021 10:38
@Shillaker Shillaker marked this pull request as ready for review October 18, 2021 16:47
@Shillaker Shillaker self-assigned this Oct 18, 2021
SPDLOG_ERROR("Cannot convert snapshot data type to string: {}", dt);
throw std::runtime_error("Cannot convert data type to string");
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure if there's a nicer way to do this. Could perhaps be done with a macro, but not sure it would be any clearer or take up fewer lines.

@Shillaker Shillaker merged commit 0b251c1 into master Oct 19, 2021
@Shillaker Shillaker deleted the cross-page-merge-regions branch October 19, 2021 14:17
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.

2 participants