Skip to content

Conversation

@sigurpol
Copy link

@sigurpol sigurpol commented Apr 4, 2025

Fix the following warning in patches.rs:

warning: call to `.clone()` on a reference in this situation does nothing
   --> src/patches.rs:119:66
    |
119 |                       .ok_or("Unable to get path from toml Value")?
    |  __________________________________________________________________^
120 | |                     .clone(),
    | |____________________________^ help: remove this redundant call
    |
    = note: the type `str` does not implement `Clone`, so calling `clone` on `&str` copies the reference, which does not do anything and can be removed
    = note: `#[warn(noop_method_call)]` on by default

By removing the ⁠.clone(), the code becomes more efficient as it avoids an unnecessary string allocation and copy. ⁠

@sigurpol sigurpol requested a review from a team as a code owner April 4, 2025 14:11
@sigurpol
Copy link
Author

sigurpol commented Apr 4, 2025

@kianenigma , since you mentioned the existence of this fork in our 1:1 yesterday or @seadanda since you have introduced me to the beauty of cargo remote in your onboarding doc... maybe you want to have a look. Pretty pointless, I know 😅

@sigurpol sigurpol force-pushed the remove-clone-on-reference branch 4 times, most recently from ee8349c to 7472b51 Compare April 4, 2025 14:34
The current `9b0c1fce7a93df8e3bb8926b0d6e9d89e92f20a7` is deprecated.
@sigurpol sigurpol force-pushed the remove-clone-on-reference branch from 7472b51 to d26d6cb Compare April 4, 2025 14:36
Copy link

@seadanda seadanda left a comment

Choose a reason for hiding this comment

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

Caching update seems fine to me too

@seadanda seadanda requested a review from alvicsam April 7, 2025 09:17
@sigurpol sigurpol merged commit eeae12c into paritytech:master Apr 7, 2025
2 checks passed
@sigurpol sigurpol deleted the remove-clone-on-reference branch April 7, 2025 11:25
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.

3 participants