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

Fix warnings #82

Merged
merged 3 commits into from
Mar 14, 2024
Merged

Fix warnings #82

merged 3 commits into from
Mar 14, 2024

Conversation

joaosa
Copy link
Collaborator

@joaosa joaosa commented Mar 12, 2024

There was a multitude of small warnings showing up when running the cli. As the majority of them were easy fixes, here's a PR to address that.

  • Removed unused imports;
  • Removed uneeded variable mutability;
  • Used shorthand notation for structs.

@joaosa joaosa requested review from travis and gammazero March 12, 2024 12:47
@travis
Copy link

travis commented Mar 12, 2024

I don't know Rust but fixing warnings seems good! @gammazero are you a more experienced Rustacean? I'll approve this but worth noting that I mostly don't really understand what these changes do...

@joaosa
Copy link
Collaborator Author

joaosa commented Mar 13, 2024

I don't know Rust but fixing warnings seems good! @gammazero are you a more experienced Rustacean? I'll approve this but worth noting that I mostly don't really understand what these changes do...

@travis I'll try to explain each one of them for future ref as well. Either way, these suggestions came directly from rust-analyzer. Hope this is helpful :)

  • Removed unused imports

There were some imports left behind and those got flagged.

The FromStr trait is implicitly used when there is a from_str call. That is not the case for the vec_address, and address mods.
As for cli/signer/src/lib.rs, I only see uses of from_string, so it's no longer used as well.

vm_shared::address::set_current_network isn't used and neither is fvm_ipld_encoding::Cbor.

  • Removed unneeded variable mutability

The keyword mut can be used to indicate a mutable variable (i.e. whose value can be rewritten) or for mutable references (i.e. to burrow a ref into another scope/function where its value is going to be modified and that change will become visible in the scope that owns that var). More here.

Neither of these cases apply to the lines that got changed. My theory is that those were left there because there was likely the intention of having variable reassignment going on (and in some languages explicitly declaring mutability is required for reassignment). In Rust, however, there is variable shadowing and that is the language feature that ended up getting used in the code I modified.

  • Used shorthand notation for structs

Just like in JS and it looks cleaner :)

@travis
Copy link

travis commented Mar 13, 2024

thanks @joaosa! :shipit:

@joaosa joaosa merged commit 447b45e into main Mar 14, 2024
1 check passed
@joaosa joaosa deleted the fix_warnings branch March 14, 2024 11:10
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