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

Reintroduce Diff::LCS::Change#to_ary #50

Merged
merged 2 commits into from Feb 2, 2019
Merged

Conversation

halostatue
Copy link
Owner

  • This required some level of code remediation in the main library, but all
    tests now pass:

    • Patchsets are now internally flattened one level explicitly, rather than
      using Array#flatten. This ensures that only the outer patchset array is
      flattened.

Fixes #48.

@halostatue
Copy link
Owner Author

@knu I believe that this fixes everything. All of the tests pass, but I’d appreciate some additional eyes on this because it is a fairly large change.

-   This required some level of code remediation in the main library, but all
    tests now pass:

    -  Patchsets are now internally flattened one level explicitly, rather than
       using Array#flatten. This ensures that only the outer patchset array is
       flattened.

Fixes #48.
@knu
Copy link
Contributor

knu commented Jan 31, 2019

Sorry for the delay, I’ll look into this later today!

@knu
Copy link
Contributor

knu commented Jan 31, 2019

Looks good, except that .flatten is still mentioned in a couple of places in the documentation, which should be updated.

I don't really get the idea of a "patchset" yet, but is it a mix of a single Change/ContextChange object or an array of them?

@halostatue
Copy link
Owner Author

Your reading of 'patchset' is correct; it is an ordered array of changes, where a group of related changes may themselves be an array of changes.

@halostatue halostatue merged commit 3a89de0 into master Feb 2, 2019
@halostatue halostatue deleted the flatten-clean-up branch February 2, 2019 03:26
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