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.

Sign up for GitHub

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

Jump to bottom

Use coercion for arguments #29

Merged
merged 30 commits into from
Jan 4, 2021
Merged

Use coercion for arguments #29

merged 30 commits into from
Jan 4, 2021

Conversation

cbreeden
Copy link
Collaborator

@cbreeden cbreeden commented Sep 23, 2016 •
edited by simonsan
Loading

Describe some of the reasons it is considered idiomatic to use &str over &String in most cases.

ref.: #27

FraGag
FraGag suggested changes Sep 24, 2016
View reviewed changes
idioms/str-vs-string.md Outdated Show resolved Hide resolved
idioms/str-vs-string.md Outdated Show resolved Hide resolved
idioms/str-vs-string.md Outdated Show resolved Hide resolved
idioms/str-vs-string.md Outdated Show resolved Hide resolved
@cbreeden cbreeden changed the title Create str-vs-string section String vs str idiom Sep 24, 2016
@cbreeden
Copy link
Collaborator Author

@lfairy Does this look fine to you?

FraGag
FraGag suggested changes Sep 24, 2016
View reviewed changes
idioms/str-vs-string.md Outdated Show resolved Hide resolved
lambda-fairy
lambda-fairy reviewed Sep 24, 2016
View reviewed changes
Copy link
Collaborator

@lambda-fairy lambda-fairy left a comment

Choose a reason for hiding this comment

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

It looks pretty much good to me, modulo the comments above.

I wonder if this article should be about borrowed types in general (e.g. talk about Vec<T> and &[T] as well), but if you think &String is common enough to deserve its own article then that's fine.

idioms/str-vs-string.md Outdated Show resolved Hide resolved
idioms/str-vs-string.md Outdated Show resolved Hide resolved
idioms/str-vs-string.md Outdated Show resolved Hide resolved
idioms/str-vs-string.md Outdated Show resolved Hide resolved
@cbreeden
Copy link
Collaborator Author

I wonder if this article should be about borrowed types in general (e.g. talk about Vec and &[T] as well)

This is a good point. Let me think about that.

nrc
nrc reviewed Sep 25, 2016
View reviewed changes
idioms/str-vs-string.md Outdated Show resolved Hide resolved
@cbreeden cbreeden changed the title String vs str idiom Using coercion for arguments Oct 8, 2016
@cbreeden
Copy link
Collaborator Author

cbreeden commented Oct 8, 2016

@nrc @lfairy Do you like this approach better?

lambda-fairy
lambda-fairy requested changes Oct 9, 2016
View reviewed changes
idioms/coercion-arguments.md Outdated Show resolved Hide resolved
idioms/coercion-arguments.md Outdated Show resolved Hide resolved
idioms/coercion-arguments.md Outdated Show resolved Hide resolved
idioms/coercion-arguments.md Outdated Show resolved Hide resolved
@simonsan simonsan added C-addition Category: Adding new content, something that didn't exist in the repository before S-review Status: A PR that is currently under review or where a review is the next step labels Dec 31, 2020
@simonsan @MarcoIeni
Update idioms/coercion-arguments.md
87985ca 
Co-authored-by: Marco Ieni <11428655+MarcoIeni@users.noreply.github.com>
@MarcoIeni
Copy link
Collaborator

Other than the 2 open suggestions I've made it looks good to me.

@simonsan @MarcoIeni
Update idioms/coercion-arguments.md
303bdba 
Co-authored-by: Marco Ieni <11428655+MarcoIeni@users.noreply.github.com>
@simonsan simonsan changed the title Using coercion for arguments Use coercion for arguments Jan 4, 2021
@simonsan @pickfire
Update idioms/coercion-arguments.md
45360aa 
Co-authored-by: Ivan Tham <pickfire@riseup.net>
lambda-fairy
lambda-fairy previously approved these changes Jan 4, 2021
View reviewed changes
Copy link
Collaborator

@lambda-fairy lambda-fairy left a comment

Choose a reason for hiding this comment

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

LGTM modulo comments

idioms/coercion-arguments.md Outdated Show resolved Hide resolved
idioms/coercion-arguments.md Outdated Show resolved Hide resolved
idioms/coercion-arguments.md Show resolved Hide resolved
idioms/coercion-arguments.md Show resolved Hide resolved
@simonsan @lambda-fairy
Update idioms/coercion-arguments.md
136b9b0 
Co-authored-by: Chris Wong <lambda.fairy@gmail.com>
@simonsan simonsan merged commit c232127 into master Jan 4, 2021
@simonsan simonsan deleted the cbreeden-str-vs-string branch January 4, 2021 08:31
@simonsan
Copy link
Collaborator

simonsan commented Jan 4, 2021

LGTM modulo comments

That was a ride 🎢 🙄

:-D

@simonsan
Copy link
Collaborator

simonsan commented Jan 4, 2021

Thanks for the PR @cbreeden ;-)

@simonsan simonsan mentioned this pull request Jan 4, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nrc nrc nrc left review comments

@pickfire pickfire pickfire left review comments

@MarcoIeni MarcoIeni MarcoIeni left review comments

@FraGag FraGag FraGag requested changes

@simonsan simonsan simonsan approved these changes

@lambda-fairy lambda-fairy lambda-fairy left review comments

Assignees
No one assigned
Labels
C-addition Category: Adding new content, something that didn't exist in the repository before S-review Status: A PR that is currently under review or where a review is the next step
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@cbreeden @simonsan @MarcoIeni @FraGag @nrc @lambda-fairy @pickfire