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

Delete New*SetFromSlice() and NewSetWith() APIs #84

Merged
merged 1 commit into from
Mar 28, 2022
Merged

Delete New*SetFromSlice() and NewSetWith() APIs #84

merged 1 commit into from
Mar 28, 2022

Conversation

jeffwidman
Copy link
Contributor

@jeffwidman jeffwidman commented Mar 27, 2022

Builds on #83 , that should be merged first, then this.

These are no longer needed now that both NewSet() and
NewThreadUnsafeSet() accept optional vals.

Removing them simplifies the API surface, so that developers aren't left
scratching their heads wondering why there's both a NewSet(vals ...T)
and a NewSetFromSlice() that do the same thing.

Previously they couldn't be removed due to backwards compatibility
concerns, but with the go 1.18 drop of generics, this library will be
cutting a new v2 release, so a perfect time to cleanup this confusing
API as well.

Fix #78

@jeffwidman jeffwidman changed the title Delete New*SetFromSlice() and NewSetWith() APIs Delete New*SetFromSlice() and NewSetWith() APIs Mar 27, 2022
@deckarep
Copy link
Owner

@jeffwidman - thanks for these contributions, I merged your previous change and this one is showing merge conflicts. Would you be up to resolving the merge conflicts and then I'll do a merge?

These are no longer needed now that both `NewSet()` and
`NewThreadUnsafeSet()` accept optional vals.

Removing them simplifies the API surface, so that developers aren't left
scratching their heads wondering why there's both a `NewSet(vals ...T)`
and a `NewSetFromSlice()` that do the same thing.

Previously they couldn't be removed due to backwards compatibility
concerns, but with the `go 1.18` drop of generics, this library will be
cutting a new `v2` release, so a perfect time to cleanup this confusing
API as well.
@jeffwidman
Copy link
Contributor Author

Thanks @deckarep, I just rebased to fix the merge conflicts. 👍

@deckarep deckarep merged commit 42e8297 into deckarep:main Mar 28, 2022
@jeffwidman jeffwidman deleted the delete-fromSlice-apis branch March 28, 2022 04:38
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.

[Question] NewSet vs NewSetWith
2 participants