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

Update structs3.rs #608

Merged
merged 1 commit into from
Feb 4, 2022
Merged

Update structs3.rs #608

merged 1 commit into from
Feb 4, 2022

Conversation

sundevilyang
Copy link
Contributor

as a totally newbie to Rust, I don't know panic statement from https://doc.rust-lang.org/book/ and rustlings in the beginning. After a hard searching of [should_panic], then I figure out panic statement.

So it's helpful to tell the learner that write a panic statement here.

as a totally newbie to Rust, I don't know panic statement from https://doc.rust-lang.org/book/ and rustlings in the beginning. After a hard searching of [should_panic], then I figure out panic statement. 

So it's helpful to tell the learner that write a panic statement here.
@jfchevrette
Copy link
Contributor

As a newcomer to rust I also got a bit confused on that one. Looking at the tests and seeing #[should_panic] gave me a hint but I don't think understanding macros and tests should be expected of new users at this point in the rustlings exercices.

As such I like the idea behind this change, however I personally think this is giving too much as I think we want the user to think about that should happen if the new function can't create a new Package object.

I think a better idea would be to hint at it without directly telling where to put the panic. For example, could we add a sentence or two at the top of the file to tell the the user to pay attention to the two functions which should return something (the ??? markers) in comparison to the new function which should return a new Package, otherwise what can be done if it can't? (and hint at: https://doc.rust-lang.org/book/ch09-01-unrecoverable-errors-with-panic.html). I think this would guide the user better into understanding when and where panics are appropriate.

@Zerotask
Copy link
Contributor

@jfchevrette
I agree with you. A hint is probably better here. Otherwise it could be too easy. There should be a little challenge and if people have to read the book to find the solution, then they will learn more.
Otherwise they could recognize panic as a macro and just use it without knowing why / what it does.

@shadows-withal shadows-withal merged commit 4f7ff5d into rust-lang:main Feb 4, 2022
@shadows-withal
Copy link
Member

@all-contributors please add @sundevilyang for content

@allcontributors
Copy link
Contributor

@diannasoreil

I've put up a pull request to add @sundevilyang! 🎉

dlip added a commit to dlip/rustlings that referenced this pull request Feb 10, 2022
* upstream/main:
  docs: update .all-contributorsrc [skip ci]
  docs: update README.md [skip ci]
  doc: Add hints on how to get gcc installed (rust-lang#741)
  docs: update .all-contributorsrc [skip ci]
  docs: update README.md [skip ci]
  fix(structs3): Add a hint for panic (rust-lang#608)
  fix(errors1): Add a comment to make the purpose more clear (rust-lang#486)
  fix(clippy1): Set clippy::float_cmp lint to deny (rust-lang#907)
  fix(intro1): Add compiler error explanation.
  feat(intro): Add intro section.
  docs(option): improve further information
ppp3 pushed a commit to ppp3/rustlings that referenced this pull request May 23, 2022
as a totally newbie to Rust, I don't know panic statement from https://doc.rust-lang.org/book/ and rustlings in the beginning. After a hard searching of [should_panic], then I figure out panic statement. 

So it's helpful to tell the learner that write a panic statement here.
dmoore04 pushed a commit to dmoore04/rustlings that referenced this pull request Sep 11, 2022
as a totally newbie to Rust, I don't know panic statement from https://doc.rust-lang.org/book/ and rustlings in the beginning. After a hard searching of [should_panic], then I figure out panic statement. 

So it's helpful to tell the learner that write a panic statement here.
Spacebody pushed a commit to Spacebody/my-rustlings that referenced this pull request Nov 18, 2022
as a totally newbie to Rust, I don't know panic statement from https://doc.rust-lang.org/book/ and rustlings in the beginning. After a hard searching of [should_panic], then I figure out panic statement. 

So it's helpful to tell the learner that write a panic statement here.
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

Successfully merging this pull request may close these issues.

4 participants