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 more complex filtering #3

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dwhenry
Copy link

@dwhenry dwhenry commented Jul 31, 2020

This PR duplicates the existing code then converts it from a module-based into a Class-based approach. The main reason for this to add extra functionality on the addition check (i.e. I want to be able to programmatically decide when to include additional based on the type of parent object and the current path)

This type of change was just not possible with the previous code implementation. I have reused the existing methods and avoided changing the interface so this can be used as a drop-in replacement for the previous code. The advantage of this approach is that rebasing changes from the master branch is do as should pick up any changes which are not to the core diffing method automatically.

@espadrine
Copy link
Owner

Hi @dwhenry!

Would it be OK to apply the changes to lib/json-diff/diff.rb?
Your code organization looks good, and it seems backwards-compatible; am I wrong?

In terms of versioning, it feels awkward to have both a v2 and a version 0.5.0-dwhenry.

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

2 participants