-
Notifications
You must be signed in to change notification settings - Fork 48
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
Finish fixing #17 #78
Conversation
Ah, yes, that's my fault because I ended up doing some squashing. If only the last two matter, run: $ git remote add upstream https://github.com/jonhoo/flurry.git
$ git fetch upstream
$ git rebase --onto upstream/master HEAD~2 That should, in theory, replay just the last two commits onto the current $ git push --force-with-lease To overwrite your current PR branch with the rebased one. BTW, if you want to learn more about this sort of magic, I highly recommend this lecture. |
The documentation hasn't beeng updated yet.
@GarkGarcia I just pushed a commit to |
The documentation still needs work
We should probably think about implementing |
…MapRef::is_superset` methods generic
@GarkGarcia Let's leave those for later so that we do not overload this PR too much. |
Fair enough. |
…hSet::is_superset` take two different guards (one for `self` and another one for `other`)
The return type does not have a lifetime, so there's no concern here about capturing the lifetime of `&Q`.
…`HashSet` predicates
Okay, so, apparently I can't push directly to your branch, but I pushed the added commits to $ git fetch upstream
$ git merge upstream/pr-17-augment |
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 👍
@GarkGarcia In a follow-up PR, it'd be great to also add a test suite for the ref version of set (just like we have one for map) |
Makes sense. We could probably include the tests in a PR for #20? |
This is a follow up from #17.
I messed something up while trying to pull some changes from upstream, so there are 23 commits from my previous contribution in here.
The only relevants commits are the last two.
This change is