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

add some more From impls + IntoCow #24

Merged
merged 4 commits into from
Oct 29, 2022
Merged

add some more From impls + IntoCow #24

merged 4 commits into from
Oct 29, 2022

Conversation

Emilgardis
Copy link
Member

No description provided.

@Emilgardis
Copy link
Member Author

related to twitch-rs/twitch_api#280

@Emilgardis Emilgardis added the release marks a pr that does a release label Oct 28, 2022
@Emilgardis Emilgardis force-pushed the convenience branch 2 times, most recently from 8b78800 to c25f8f5 Compare October 28, 2022 18:02
@Emilgardis Emilgardis force-pushed the convenience branch 2 times, most recently from 0085dda to 7fbdaad Compare October 28, 2022 22:20
Comment on lines +168 to +195
pub fn broadcaster_id<'a>(broadcaster_id: impl IntoCow<'a, UserIdRef> + 'a) -> bool {
struct K<'a> {
id: std::borrow::Cow<'a, UserIdRef>,
}
let k = K {
id: broadcaster_id.to_cow(),
};
matches!(k.id, std::borrow::Cow::Borrowed(_))
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Nerixyz I think this solves the issue described in twitch-rs/twitch_api#280 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this should solve it 👍

@Emilgardis Emilgardis changed the title add some more From impls add some more From impls + IntoCow Oct 28, 2022
@Emilgardis Emilgardis force-pushed the convenience branch 2 times, most recently from fb0b663 to 0cc5242 Compare October 28, 2022 22:33
@Emilgardis
Copy link
Member Author

bors r+

bors bot added a commit that referenced this pull request Oct 29, 2022
24: add some more `From` impls + `IntoCow` r=Emilgardis a=Emilgardis



Co-authored-by: Emil Gardström <[email protected]>
@Emilgardis
Copy link
Member Author

bors r-

@bors
Copy link
Contributor

bors bot commented Oct 29, 2022

Canceled.

@Emilgardis
Copy link
Member Author

bors r+

bors bot added a commit that referenced this pull request Oct 29, 2022
24: add some more `From` impls + `IntoCow` r=Emilgardis a=Emilgardis



Co-authored-by: Emil Gardström <[email protected]>
@bors
Copy link
Contributor

bors bot commented Oct 29, 2022

Build failed:

@Emilgardis
Copy link
Member Author

bors r+

@bors
Copy link
Contributor

bors bot commented Oct 29, 2022

Build succeeded:

  • audit
  • CI

@bors bors bot merged commit 547c624 into main Oct 29, 2022
@bors bors bot deleted the convenience branch October 29, 2022 12:53
@spikespaz
Copy link

Isn't this an antipattern? The implementation isn't complete and is causing build errors. Can't we expect the API user to wrap the types in Cow themselves, instead of making your source code harder to read.

@Emilgardis
Copy link
Member Author

@spikespaz can you please elaborate in a new issue. There shouldn't be any problems with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release marks a pr that does a release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants