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

Clean up and rename the HashSet iterators #19993

Merged
merged 1 commit into from
Dec 22, 2014
Merged

Conversation

bluss
Copy link
Member

@bluss bluss commented Dec 18, 2014

This removes the type SetAlgebraItems and replaces it with the
structs Intersection and Difference.

Rename the existing HashSet iterators according to RFC #344:

  • SetItems -> Iter
  • SetMoveItems -> IntoIter
  • Remaining set combination iterators renamed to Union and SymmetricDifference

@bluss
Copy link
Member Author

bluss commented Dec 18, 2014

So first commit -- what can be done.

Second commit -- is mildly breaking and what I want to be done. :)

@Gankra
Copy link
Contributor

Gankra commented Dec 18, 2014

r? @huonw (will do myself in a few days if you don't)

@Gankra
Copy link
Contributor

Gankra commented Dec 19, 2014

Wait I lied I did it in 8 hours. I'm fine with this breaking change because we need to rename basically every collection iterator anyway. Actually maybe we should just nip that in the bud right here: I believe all these iterators should be named after the method that creates them. SetItems -> Iter, DifferenceItems -> Difference, etc.

@Gankra
Copy link
Contributor

Gankra commented Dec 19, 2014

For reference: https://github.com/rust-lang/rfcs/blob/master/text/0344-conventions-galore.md#iterator-type-names

(man it would be super cool if some real strong and brave developer went and implemented that...)

@csouth3
Copy link
Contributor

csouth3 commented Dec 19, 2014

If @bluss would be interested in renaming the iterators in HashSet as part of this PR, I'd be willing to go through the rest of collections and do the rest of the renaming work.

@Gankra
Copy link
Contributor

Gankra commented Dec 19, 2014

@csouth3 This user on Reddit has expressed interest in maybe taking it on as a learning project: http://www.reddit.com/r/rust/comments/2prhid/rip_treemap_treeset_triemap_trieset_lrucache_and/cmziw9c

You can collaborate with or mentor them on it. Since it's not necessarily a hard job (just really tedious), I'm inclined to give the newer contributor priority for learning and growth reasons. If we don't hear from them in a few days feel free to take point, though.

@csouth3
Copy link
Contributor

csouth3 commented Dec 19, 2014

Ah yes, please defer to the redditor (I'm also willing to mentor if desired). Otherwise, I'll just sit back and keep an eye on it for a few days :)

@Gankra
Copy link
Contributor

Gankra commented Dec 19, 2014

@MrFloya is the reddit user. They have just contacted me on irc and are on the job! 💃

@csouth3
Copy link
Contributor

csouth3 commented Dec 19, 2014

Sweet 😎

@bluss
Copy link
Member Author

bluss commented Dec 19, 2014

I'll do just the HashSet iterators and push a commit for that to this PR

@bluss
Copy link
Member Author

bluss commented Dec 19, 2014

Updated to rename all HashSet iterators. I think I'd like to squash the three commits together after review.

@Gankra
Copy link
Contributor

Gankra commented Dec 19, 2014

r=me with squash

This removes the type SetAlgebraItems and replaces it with the
structs Intersection and Difference.

Rename the existing HashSet iterators according to RFC rust-lang#344:

* SetItems -> Iter
* SetMoveItems -> IntoIter
* Remaining set combination iterators renamed to Union and SymmetricDifference

[breaking-change]
@bluss bluss changed the title Clean up the last HashSet iterators Clean up and rename the HashSet iterators Dec 19, 2014
@bluss
Copy link
Member Author

bluss commented Dec 19, 2014

squashed and thank you

@Gankra
Copy link
Contributor

Gankra commented Dec 19, 2014

@bluss Just occurred to me to ask: You made sure no one in all of the repo relies on the name of these types?

@bluss
Copy link
Member Author

bluss commented Dec 19, 2014

Yes, it's still compiling, but I did grep around.

@Gankra
Copy link
Contributor

Gankra commented Dec 19, 2014

Good enough for me :D

@bluss
Copy link
Member Author

bluss commented Dec 19, 2014

You and me are mere mortals, bors has the final word :)

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Dec 21, 2014
This removes the type SetAlgebraItems and replaces it with the
structs Intersection and Difference.

Rename the existing HashSet iterators according to RFC rust-lang#344:

* SetItems -> Iter
* SetMoveItems -> IntoIter
* Remaining set combination iterators renamed to Union and SymmetricDifference
@bors bors merged commit cf350ea into rust-lang:master Dec 22, 2014
@bluss bluss deleted the setalgebraitems branch December 22, 2014 08:32
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.

4 participants