kevaundray opened PR #6203 from kevaundray:kw/clippy-suggestions
to bytecodealliance:main
:
<!--
Made a few clippy fixes in this PR. I did not do all of the clippy fixes as it may be that only certain things should be changed and or clippy fixes are not deemed necessary.
This PR is more of a temperature check :)
Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
kevaundray requested jameysharp for a review on PR #6203.
kevaundray requested wasmtime-compiler-reviewers for a review on PR #6203.
kevaundray requested wasmtime-core-reviewers for a review on PR #6203.
kevaundray edited PR #6203:
Made a few clippy fixes in this PR. I did not do all of the clippy fixes as it may be that only certain things should be changed and or clippy fixes are not deemed necessary.
This PR is more of a temperature check :)
jameysharp submitted PR review.
jameysharp submitted PR review.
jameysharp created PR review comment:
In this specific case I think the original structure was more clear, so I'd prefer to drop this change from the PR.
jameysharp created PR review comment:
I think I'd prefer for this to call
clone
, or possiblyto_string
, rather thanto_owned
.
jameysharp created PR review comment:
We don't have consensus about the use of
ref
in this project. I personally tend to remove pairedref
and&
from patterns. But others disagree, and I certainly agree with them that explicitly writingref
allows the reader to not have to reason about as much of the program to figure out which things are references.As a result, I think we should disable this particular Clippy lint and remove this change from this PR. We'll continue to decide which style to use on a case-by-case basis.
jameysharp created PR review comment:
Let's remove this
&
/ref
change from this PR.
jameysharp created PR review comment:
As long as we're changing this, let's use
strip_prefix
:if let Some(s) = ident.0.strip_prefix('$') {
jameysharp created PR review comment:
Let's remove this
&
/ref
change from this PR.
jameysharp created PR review comment:
Let's remove this
&
/ref
change from this PR.
jameysharp created PR review comment:
Converting
match
toif
helps a lot, but let's put the&
/ref
back in this pattern.
jameysharp created PR review comment:
Let's remove this
&
/ref
change from this PR.
jameysharp created PR review comment:
Let's remove this
&
/ref
change from this PR.
jameysharp created PR review comment:
Let's remove this
&
/ref
change from this PR.
kevaundray updated PR #6203.
kevaundray updated PR #6203.
kevaundray submitted PR review.
kevaundray created PR review comment:
Thats fair -- in my code, I usally bias towards
to_owned
orclone
since its common for folks to refer to a string as either &str/str or String.Changed it to clone!
jameysharp submitted PR review.
jameysharp merged PR #6203.
Last updated: Dec 23 2024 at 12:05 UTC