Skip to content

Conversation

@cchiw
Copy link
Collaborator

@cchiw cchiw commented Feb 10, 2023

This code had variables in the function arguments that were not being used (yet).
To address that, in this PR I use the function arguments in a print statement. This changes allow us to remove the line to silence warnings for unused variables #71.

Alternatively, we could comment out the functions. As seen in #389.

Remove unused code warning #71
@cchiw cchiw changed the title Update luhn.md Don't silence warnings in credit card exercise Feb 13, 2023
@cchiw cchiw changed the title Don't silence warnings in credit card exercise Remove silence warnings in credit card exercise Feb 13, 2023
@cchiw cchiw enabled auto-merge (squash) March 2, 2023 04:50
@cchiw cchiw requested a review from mgeisler March 2, 2023 04:50
@cchiw cchiw changed the title Remove silence warnings in credit card exercise Remove silence warnings in exercises Mar 6, 2023
@cchiw cchiw marked this pull request as draft March 6, 2023 19:31
auto-merge was automatically disabled March 6, 2023 19:31

Pull request was converted to draft

@cchiw cchiw marked this pull request as ready for review March 6, 2023 19:41
@cchiw cchiw marked this pull request as draft March 6, 2023 19:59
@cchiw cchiw marked this pull request as ready for review March 7, 2023 05:11
@cchiw cchiw enabled auto-merge (squash) March 11, 2023 22:15
#: src/exercises/day-2/health-statistics.md:13
msgid ""
"```rust,should_panic\n"
"// TODO: remove this when you're done with your implementation.\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't modify the .po files — only the translators can update those.


impl User {
pub fn new(name: String, age: u32, weight: f32) -> Self {
println!("Use name {:?} age {:?} and weight{:?}", name, age, weight);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've tried to inline the format strings everywhere else in the course:

Suggested change
println!("Use name {:?} age {:?} and weight{:?}", name, age, weight);
println!("Use name {name:?} age {age:?} and weight {weight:?}");

I also fixed a space, which makes me think that you've not looked at this in the browser after making the change?

}

pub fn name(&self) -> &str {
println!("Use name {:?}", self.name);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know if we've agreed that this is a good way to "fix" the warnings? I would have added such print statements already if this is a nice way to solve it 🙂

@mgeisler
Copy link
Collaborator

Hi @cchiw,

I don't think adding a lot of print("Use {foo}") lines is a good way to silence the warnings. Why not? Because it not how actual code looks like. Let's discuss on #71.

@cchiw cchiw closed this Apr 20, 2023
auto-merge was automatically disabled April 20, 2023 06:03

Pull request was closed

@cchiw cchiw deleted the cchiw-patch-11 branch April 20, 2023 06:03
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.

2 participants