Skip to content

Make diffing stack safe #15

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

Merged
merged 5 commits into from
Jun 13, 2022
Merged

Make diffing stack safe #15

merged 5 commits into from
Jun 13, 2022

Conversation

mrdziuban
Copy link
Contributor

Fixes #14

  • Adds minimal version of cats.Eval which guarantees stack safety when using map/flatMap
  • Updates all methods in XmlDiffComputer to wrap their results in Eval
  • Sets the JVM stack size when running tests to 256K and adds a test to verify that diffing elements with a lot of children does not throw a StackOverflowError

@mrdziuban
Copy link
Contributor Author

@andyglow is there any chance you could review this and potentially release a new version if/when it's merged? Let me know if there's any more info or clarification I can provide

@andyglow
Copy link
Owner

No, no questions..
This looks great. Thank you for your contribution.
Except... May I ask you to hide Eval machinery making it a package private?

@mrdziuban
Copy link
Contributor Author

Sure thing @andyglow, just pushed another commit to add private[diff]

Copy link
Owner

@andyglow andyglow left a comment

Choose a reason for hiding this comment

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

lgtm

@andyglow andyglow merged commit 2ffdd56 into andyglow:master Jun 13, 2022
@mrdziuban mrdziuban deleted the stack-safety branch June 13, 2022 18:20
@andyglow
Copy link
Owner

hey @mrdziuban
3.0.1 has been published to sonatype
should be available in maven central soon

@mrdziuban
Copy link
Contributor Author

Thank you!

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.

Diffing is not stack safe
2 participants