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

Rename first argument Option::unwrap_or to default #65492

Closed
timClicks opened this issue Oct 17, 2019 · 3 comments · Fixed by #65638
Closed

Rename first argument Option::unwrap_or to default #65492

timClicks opened this issue Oct 17, 2019 · 3 comments · Fixed by #65638
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@timClicks
Copy link
Contributor

We currently use the definition for Option::unwrap_or:

pub fn unwrap_or(self, def: T) -> T

I sometimes read def as define. It would be clearer for me (and hopefully others!) if it had the following signature:

pub fn unwrap_or(self, default: T) -> T
@jonas-schievink jonas-schievink added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Oct 17, 2019
@petosorus
Copy link

Hi ! I'm interesting to work on this if no one has started.

@timClicks
Copy link
Contributor Author

Wow, I half expected that this would be closed as wontfix. @petosorus I am happy to submit a patch, but if you want to that would be great.

@petosorus
Copy link

Lol.
Go ahead!

@dsincl12 dsincl12 mentioned this issue Oct 20, 2019
Merged
Centril added a commit to Centril/rust that referenced this issue Oct 20, 2019
@Centril
Rollup merge of rust-lang#65638 - dsincl12:master, r=Centril
5a29bd9 
Rename the default argument 'def' to 'default'

Fixes: rust-lang#65492
@bors bors closed this as completed in 2a5c31c Oct 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Rename the default argument 'def' to 'default' dsincl12/rust
3 participants
@timClicks @jonas-schievink @petosorus