-
Notifications
You must be signed in to change notification settings - Fork 236
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
Fix change detection #643
Fix change detection #643
Conversation
The change detection failed on maps, this introduces the use of go-cmp which is better suited to this purpose. Credit to https://github.com/gdavison for providing the tests.
Can this get reviewed and merged in? |
any news on this one? thanks |
This is very concerning |
Redirecting question to @paddycarver who seems to be most active last couple of months in this repository. :) |
@kmruddy @pkolyvas-hashicorp Hi gents, I really enjoyed your Terraform 0.15 presentation today, so I hope you don't mind me tagging you here. As you will see from the comments above, it looks like merging this PR would unblock #617, which would in turn unblock hashicorp/terraform-provider-aws#15952, which is causing us a lot of pain every day. Is there anything you can do to help? Thanks! |
@deyvsh I’ve been reading more about go-cmp and I don’t think it’s the correct approach due to its performance implications. I think this is why the PR is not getting reviewed. I have tried an alternate approach to fix the issue in the AWS provider itself see hashicorp/terraform-provider-aws#17610 |
Hi @grahamhar 👋 Thank you for submitting this and apologies for the delayed review. I apologize that I did not see this pull request was similar to #711 before merging that in recently. Looking at the two sets of changes, #711 also updated The fix should be included in the next Terraform Plugin SDK release 🔜 . Thanks again for the contribution. |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
The change detection failed on maps, this introduces the use of go-cmp
which is better suited to this purpose.
It should fix #617 and the related aws provider issue hashicorp/terraform-provider-aws#15952
Credit to @gdavison for providing the tests.