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

Rename matcher Contains #3

Closed
povilasb opened this issue Oct 6, 2018 · 4 comments
Closed

Rename matcher Contains #3

povilasb opened this issue Oct 6, 2018 · 4 comments

Comments

@povilasb
Copy link

povilasb commented Oct 6, 2018

I'd like to bring the original issue again: ujh#45
I made a PR for it: ujh#50
Would you be interested in merging? :)

@Valloric
Copy link
Owner

Valloric commented Oct 6, 2018

Thanks for interest in contributing! :)

This seems like it might not be worth breaking backwards compatibility for. I have a rather draconian view against API breakage which is basically never, ever break it unless absolutely necessary. This doesn't seem to pass that bar for me, sorry.

Ideally we'd be able to find a way that contains can take both a single item and a vector for multiple items. This might be doable (not sure) by making the param take a special type that can be auto-converted from both a vec of Ts and a single T. @jwilm You know rust better than I do; is this doable?

@jwilm
Copy link

jwilm commented Oct 7, 2018

Something like this might work depending on how the contains API is used in practice. There is a type inference issue that you can see by removing the hint in main. If it doesn't break the test suite, then maybe it's fine?

@Valloric
Copy link
Owner

@jwilm I like this! I'd go with "if it doesn't break the test suite, it should be good."

@povilasb If you sent a PR with the implementation that @jwilm provided a prototype of, that would have a high chance of being merged.

@povilasb
Copy link
Author

Okey, I might make some spare time this week ;)

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

No branches or pull requests

3 participants