filecoin-saturn / contracts

contracts
6 stars 0 forks source link

Fix warnings #82

Closed joaosa closed 4 months ago

joaosa commented 4 months ago

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.

travis commented 4 months ago

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 commented 4 months ago

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 :)

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.

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.

Just like in JS and it looks cleaner :)

travis commented 4 months ago

thanks @joaosa! :shipit: