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

core: don't compare map diffs for computed values #7249

Merged
merged 1 commit into from
Jun 27, 2016
Merged

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Jun 20, 2016

Adding a failing test before we decide how to fix this.

issue #7244

@jbardin jbardin changed the title WIP: core: don't compare map diffs for computed values core: don't compare map diffs for computed values Jun 24, 2016
@jbardin
Copy link
Member Author

jbardin commented Jun 24, 2016

This will remove all computed map values from the InstanceDiff.Same check. That function (and our diff process) needs some close review in general, but I wanted to add this with the minimal change set possible.

This prevents spurious "diffs didn't match during apply" errors caused by computed map values.

Just like computed sets, computed maps may have both different values
and different cardinality after they're computed. Remove the computed
maps and the values from the compared diffs.
@phinze
Copy link
Contributor

phinze commented Jun 27, 2016

Nice catch here. LGTM

@jbardin jbardin merged commit b80c2f7 into master Jun 27, 2016
@jbardin jbardin deleted the jbardin/GH-7244 branch June 27, 2016 15:20
@ghost
Copy link

ghost commented Apr 24, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants