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

ResourceData: create variadic 'HasChanges' method #241

Merged
merged 1 commit into from
Nov 9, 2019

Conversation

appilon
Copy link
Contributor

@appilon appilon commented Nov 8, 2019

I'm open to bikeshedding on the function name. Strongest alternative probably HasChanges (this alternative just "feels better" than HaveChange I may switch if there is support for it)? Voted on HasChanges Practically speaking we could have switched HasChange itself to be variadic and it likely wouldn't have broken anything in the real world, however from a compiled language/type signature point of view anyone who has assigned this method (unlikely) would experience a breaking change (the rare L for compiled languages).

Also I'm allowing a noop call d.HasChanges() I wasn't sure maybe it should panic without a single key? Could change signature to

func (d *ResourceData) HasChanges(firstKey string, others ...string) bool

Closes #194

@appilon appilon requested review from a team November 8, 2019 16:08
@appilon appilon changed the title ResourceData: create variadic has change method ResourceData: create variadic 'HasChange' method Nov 8, 2019
@appilon appilon force-pushed the variadic-has-change branch 2 times, most recently from 7727111 to 5017177 Compare November 8, 2019 21:11
@appilon appilon changed the title ResourceData: create variadic 'HasChange' method ResourceData: create variadic 'HasChanges' method Nov 8, 2019
Copy link
Contributor

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rubber LGTM 👍

@ghost
Copy link

ghost commented Mar 31, 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 Mar 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

d.HasChanges("", "", "")
2 participants