-
Couldn't load subscription status.
- Fork 18
ran cargo clippy --fix -- -Wclippy::use_self
#172
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request refactors multiple source files in the gh-workflow crate to replace explicit type names with the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes While the refactoring spans 8 files, each change follows a consistent, low-risk pattern of replacing explicit type names with Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/gh-workflow/src/ctx.rs (1)
59-87: Parameter type changes are correct.The replacement of explicit type with
Selfin theotherparameter is appropriate and improves code consistency.Consider moving instead of cloning for consistency.
Since
otheris owned (taken by value), you could moveother.stepdirectly instead of cloning it ineq,and, andormethods. This would be consistent with theconcatmethod (line 96) and eliminate unnecessary clones:pub fn eq(&self, other: Self) -> Context<bool> { Context { marker: Default::default(), step: Step::Eq { left: Box::new(self.step.clone()), - right: Box::new(other.step.clone()), + right: Box::new(other.step), }, } }Apply the same pattern to
andandormethods.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
crates/gh-workflow/src/cargo.rs(1 hunks)crates/gh-workflow/src/container.rs(1 hunks)crates/gh-workflow/src/ctx.rs(7 hunks)crates/gh-workflow/src/env.rs(3 hunks)crates/gh-workflow/src/job.rs(1 hunks)crates/gh-workflow/src/release_plz.rs(2 hunks)crates/gh-workflow/src/rust_flag.rs(2 hunks)crates/gh-workflow/src/step.rs(6 hunks)crates/gh-workflow/src/toolchain.rs(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: tusharmath
PR: tailcallhq/gh-workflow#97
File: crates/gh-workflow-tailcall/src/workflow.rs:121-121
Timestamp: 2024-12-10T06:33:21.046Z
Learning: In `crates/gh-workflow-tailcall/src/workflow.rs`, prefer converting standalone functions into methods of the `Workflow` struct when appropriate.
🧬 Code graph analysis (2)
crates/gh-workflow/src/cargo.rs (4)
crates/gh-workflow/src/job.rs (1)
new(77-83)crates/gh-workflow/src/step.rs (1)
new(199-204)crates/gh-workflow/src/expression.rs (1)
new(13-15)crates/gh-workflow/src/workflow.rs (1)
new(113-115)
crates/gh-workflow/src/rust_flag.rs (1)
crates/gh-workflow/src/env.rs (2)
from(17-19)from(48-52)
🔇 Additional comments (31)
crates/gh-workflow/src/job.rs (1)
25-26: LGTM! Consistent use ofSelfin constructor.The change correctly applies the
use_selflint suggestion in theFromimplementation.crates/gh-workflow/src/env.rs (4)
17-18: LGTM! Proper use ofSelfinFromimplementation.
24-30: LGTM! Consistent constructor pattern.
34-35: LGTM! Proper use ofSelf::default().
48-51: LGTM! Consistent use ofSelfin tuple conversion.crates/gh-workflow/src/container.rs (1)
81-84: LGTM! Consistent struct construction withSelf.crates/gh-workflow/src/release_plz.rs (2)
52-55: LGTM! Consistent use ofSelfin pattern matching.
66-70: LGTM! Consistent use ofSelfin pattern matching.crates/gh-workflow/src/toolchain.rs (7)
19-23: LGTM! Consistent use ofSelfinVersionDisplay.
28-29: LGTM! Proper constructor pattern.
42-46: LGTM! Consistent use ofSelfinComponentDisplay.
61-67: LGTM! Consistent use ofSelfinArchDisplay.
80-85: LGTM! Consistent use ofSelfinVendorDisplay.
99-105: LGTM! Consistent use ofSelfinSystemDisplay.
119-125: LGTM! Consistent use ofSelfinAbiDisplay.crates/gh-workflow/src/cargo.rs (1)
28-35: LGTM! Consistent use ofSelfin constructor.Both the return type and construction properly use
Self.crates/gh-workflow/src/step.rs (7)
75-76: LGTM! Proper use ofSelfinFromimplementation.
164-165: LGTM! Consistent struct construction.
169-183: LGTM! Consistent struct construction.
199-203: LGTM! Proper use ofSelfin constructor.
225-226: LGTM! Consistent use ofSelfin return type.
246-250: LGTM! Proper use ofSelfin tuple conversion.
254-255: LGTM! Consistent use ofSelfin return type and construction.crates/gh-workflow/src/rust_flag.rs (4)
26-29: LGTM! Proper use ofSelfin trait implementation.
34-52: LGTM! Consistent constructor pattern across all helper methods.
56-68: LGTM! Consistent use ofSelfin pattern matching.
72-76: LGTM! Proper use ofSelf::from()in conversion.crates/gh-workflow/src/ctx.rs (4)
45-47: LGTM! Idiomatic use ofSelfin constructor.The replacement of
ContextwithSelfis correct and improves consistency.
91-99: LGTM! Consistent use ofSelfthroughout.The method signature and constructor correctly use
Self, and the implementation efficiently movesother.stepsinceotheris owned.
199-227: LGTM! Pattern matching correctly updated to useSelf.The replacement of
Step::withSelf::in all match arms is correct and makes the code more maintainable.
235-248: LGTM! From implementations correctly useSelf.Both trait implementations properly replace explicit type names with
Self, improving consistency across the codebase.
Summary by CodeRabbit