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

Rephrase for clarity #3809

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

octavioooooo
Copy link

Didn't see any explicit standards in CONTRIBUTING.md, so I apologize, but this is my first PR here. Wanted to start small.

@carols10cents
Copy link
Member

Can you describe what you find unclear about the original and how the change clarifies it for you?

@octavioooooo
Copy link
Author

Sure @carols10cents ! Thank you for having a look.

  1. "i32 is Copy": isn't this a confusing thing to say? I thought i32 was a type, but now it's also a trait? Isn't "i32 implements the Copy trait" more accurate?
  2. "x would move into the function": this one is a personal preference. The previous version has a hypothetical scenario ("would") followed by a contradiction ("but"). I think my small brain would appreciate if the causality ("i32 implements Copy -> x does not move into function") was simpler to follow and written out.

@StefanSalewski
Copy link

"i32 is Copy": isn't this a confusing thing to say?

"is copy" is a Rust term. I read a short explanation when I started learning Rust a few months ago -- I am not sure if in the book or elsewhere. Can not find it in the book currently -- of course explanation should come before use in the book. You can read explanation here: https://dhghomon.github.io/easy_rust/Chapter_19.html

@octavioooooo
Copy link
Author

"is copy" is a Rust term.

I didn't know about this "Easy Rust" book. TIL simple types with known sizes are always copied and therefore are called "Copy Types". Writing "i32 is Copy" seems to be a "Rust thing" and isn't confusing at all then haha.

Thank you. Should I close this PR or is it still valid?

@carols10cents
Copy link
Member

This is still valid-- please leave it open. I do need to think about how exactly I want to update this though, and it might be a while before I get to it.

I actually tried to remove all instances of "is [Trait]" a while ago but looks like you found one I missed! Thank you for the great catch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants