-
Notifications
You must be signed in to change notification settings - Fork 281
Support 1.23 iterator for set #140
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
Conversation
FWIW: I'd really like to see this. |
@freeformz - a fresh PR would be nice. Please factor in the discussions up until now and I'll make a commitment to review it and merge of it if I can get a sign input by a few folks. |
Hi @freeformz, @deckarep. The PR was in draft because it’s still under discussion in #141. If @deckarep thinks that the discussion is finalized and a decision is reached, feel free to comment or review the PR |
@amikai and @freeformz - it looks like you both want your PR’s merged on Iterator support for this package. I appreciate both of your efforts on this but both PR’s are quite different. freeformz looks to be a significant change at the moment while amikai’s is much smaller in scope and looks to be much more straightforward. Let me be be clear: I want to support iterators but when this was initially proposed it felt too early to just jump in without a better understanding of the best way forward. This is why I didn’t rush to merge it. It’s also why I don’t rush to merges everyone’s changes. Some significant projects are using this package and any abrupt changes which cause confusion, frustration will cause these projects to quickly fork and/or abandon this repo. So as it stands I’m interested in taking the least breaking changes, with the least scope currently. Additionally, I want to ensure that methods are deprecated overtime and not overnight. So with all of that said, @amikai - it seems like the last thing we had to come to agreement on was just the naming of the iterator method? And it looks like @freeformz - your input is still welcome but I want to introduce smaller scopes changes at this time. As more people migrate to later versions of Go I will consider cutting a V3 release. |
No worries, all good. I am just really motivated to have iterator support atm so I don't really care whose implementation is accepted. I'll close my pr. TY! |
Use the name "Element" as it is more intuitive.
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.
LGTM
This PR use to discussion.