Stream: git-wasmtime

Topic: wasmtime / PR #6203 Add clippy suggestions

Wasmtime GitHub notifications bot (Apr 12 2023 at 18:16):

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.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Apr 12 2023 at 18:17):

kevaundray requested jameysharp for a review on PR #6203.

Wasmtime GitHub notifications bot (Apr 12 2023 at 18:17):

kevaundray requested wasmtime-compiler-reviewers for a review on PR #6203.

Wasmtime GitHub notifications bot (Apr 12 2023 at 18:17):

kevaundray requested wasmtime-core-reviewers for a review on PR #6203.

Wasmtime GitHub notifications bot (Apr 12 2023 at 18:17):

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

Wasmtime GitHub notifications bot (Apr 12 2023 at 19:16):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Apr 12 2023 at 19:16):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Apr 12 2023 at 19:16):

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.

Wasmtime GitHub notifications bot (Apr 12 2023 at 19:16):

jameysharp created PR review comment:

I think I'd prefer for this to call clone, or possibly to_string, rather than to_owned.

Wasmtime GitHub notifications bot (Apr 12 2023 at 19:16):

jameysharp created PR review comment:

We don't have consensus about the use of ref in this project. I personally tend to remove paired ref and & from patterns. But others disagree, and I certainly agree with them that explicitly writing ref 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.

Wasmtime GitHub notifications bot (Apr 12 2023 at 19:16):

jameysharp created PR review comment:

Let's remove this &/ref change from this PR.

Wasmtime GitHub notifications bot (Apr 12 2023 at 19:16):

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('$') {

Wasmtime GitHub notifications bot (Apr 12 2023 at 19:16):

jameysharp created PR review comment:

Let's remove this &/ref change from this PR.

Wasmtime GitHub notifications bot (Apr 12 2023 at 19:16):

jameysharp created PR review comment:

Let's remove this &/ref change from this PR.

Wasmtime GitHub notifications bot (Apr 12 2023 at 19:16):

jameysharp created PR review comment:

Converting match to if helps a lot, but let's put the &/ref back in this pattern.

Wasmtime GitHub notifications bot (Apr 12 2023 at 19:16):

jameysharp created PR review comment:

Let's remove this &/ref change from this PR.

Wasmtime GitHub notifications bot (Apr 12 2023 at 19:16):

jameysharp created PR review comment:

Let's remove this &/ref change from this PR.

Wasmtime GitHub notifications bot (Apr 12 2023 at 19:16):

jameysharp created PR review comment:

Let's remove this &/ref change from this PR.

Wasmtime GitHub notifications bot (Apr 15 2023 at 17:00):

kevaundray updated PR #6203.

Wasmtime GitHub notifications bot (Apr 15 2023 at 17:05):

kevaundray updated PR #6203.

Wasmtime GitHub notifications bot (Apr 15 2023 at 17:08):

kevaundray submitted PR review.

Wasmtime GitHub notifications bot (Apr 15 2023 at 17:08):

kevaundray created PR review comment:

Thats fair -- in my code, I usally bias towards to_owned or clone since its common for folks to refer to a string as either &str/str or String.

Changed it to clone!

Wasmtime GitHub notifications bot (Apr 17 2023 at 15:53):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Apr 17 2023 at 16:26):

jameysharp merged PR #6203.

Last updated: Nov 22 2024 at 16:03 UTC