-
Notifications
You must be signed in to change notification settings - Fork 44
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
Detailed diff use minimal edit distance for list attribute diffs #2863
Detailed diff use minimal edit distance for list attribute diffs #2863
Conversation
This change is part of the following stack: Change managed by git-spice. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2863 +/- ##
==========================================
+ Coverage 67.50% 67.53% +0.02%
==========================================
Files 327 327
Lines 41743 41792 +49
==========================================
+ Hits 28180 28224 +44
- Misses 11977 11982 +5
Partials 1586 1586 ☔ View full report in Codecov by Sentry. |
...est/testdata/TestSDKv2DetailedDiffList/list_attribute_force_new/long_list_added_front.golden
Show resolved
Hide resolved
.../diff_test/testdata/TestSDKv2DetailedDiffList/list_attribute/list_element_added_front.golden
Show resolved
Hide resolved
What is the small modification can you add it for review please? |
Here is the godifft modification: t0yv0/godifft@main...VenelinMartinov:godifft:main I've exposed the indices of the changes so that we can use these directly instead of having to match them back to the inputs. |
9eb5075
to
78de7f4
Compare
5beda21
to
32e3548
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be good enough. Let's try it.
This PR uses a minimal edit distance algorithm to compute a sensible list attribute diff.
Uses https://github.com/t0yv0/godifft with a small modification.
Note that this is only done for list attributes as blocks might trigger replaces from a nested property. This matches the terraform behaviour, more details in #2295 (comment)
Alternative to #2862
fixes #2295
fixes #2239