docs: fix incorrect import statement in usage example #166
Conversation
WalkthroughProject-wide update to derive_setters usage: multiple public structs now include the borrow_self option in their #[setters(...)] attributes, altering generated setter receivers to borrow self. Additionally, a README import path was corrected from gh_workflows to gh_workflow. No runtime logic or control-flow changes. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/gh-workflow-tailcall/src/standard.rs (2)
73-73: Fix E0277 by converting chained builder calls to mutable locals
init_job (crates/gh-workflow-tailcall/src/standard.rs:73)
- let mut job = Job::new(name).permissions(Permissions::default().contents(Level::Read)); + let mut perms = Permissions::default(); + perms.contents(Level::Read); + let mut job = Job::new(name).permissions(perms);release_job (crates/gh-workflow-tailcall/src/standard.rs:138–141)
- .concurrency( - Concurrency::new(Expression::new("release-${{github.ref}}")) - .cancel_in_progress(false), - ) + .concurrency({ + let mut conc = Concurrency::new(Expression::new("release-${{github.ref}}")); + conc.cancel_in_progress(false); + conc + })autofix branch (crates/gh-workflow-tailcall/src/standard.rs:158–161)
- job.concurrency( - Concurrency::new(Expression::new("autofix-${{github.ref}}")) - .cancel_in_progress(false), - ) + job.concurrency({ + let mut conc = Concurrency::new(Expression::new("autofix-${{github.ref}}")); + conc.cancel_in_progress(false); + conc + })
166-177: Restructure Toolchain/Cargo builder chains to move owned values into add_step/add_step_when.
borrow_self makes setters return &mut; chained temporaries can no longer be moved. Use localmutvars, then pass owned values.Apply these diffs:
@@ - job.add_step( - Toolchain::default() - .add_nightly() - .add_clippy() - .add_fmt() - .cache(true) - .cache_directories(vec![ - "~/.cargo/registry".into(), - "~/.cargo/git".into(), - "target".into(), - ]), - ) + let mut tc = Toolchain::default().add_nightly().add_clippy().add_fmt(); + tc.cache(true); + tc.cache_directories(vec![ + "~/.cargo/registry".into(), + "~/.cargo/git".into(), + "target".into(), + ]); + job.add_step(tc) @@ - .add_step( - Cargo::new("fmt") - .name("Cargo Fmt") - .nightly() - .add_args("--all") - .add_args_when(!auto_fix, "--check"), - ) + .add_step({ + let mut fmt = Cargo::new("fmt"); + fmt.name("Cargo Fmt"); + fmt.nightly(); + fmt.add_args("--all"); + fmt.add_args_when(!auto_fix, "--check"); + fmt + }) @@ - .add_step( - Cargo::new("clippy") - .name("Cargo Clippy") - .nightly() - .add_args_when(auto_fix, "--fix") - .add_args_when(auto_fix, "--allow-dirty") - .add_args("--all-features --workspace -- -D warnings"), - ) + .add_step({ + let mut clippy = Cargo::new("clippy"); + clippy.name("Cargo Clippy"); + clippy.nightly(); + clippy.add_args_when(auto_fix, "--fix"); + clippy.add_args_when(auto_fix, "--allow-dirty"); + clippy.add_args("--all-features --workspace -- -D warnings"); + clippy + }) @@ - .add_step_when( - matches!(self.test_runner, TestRunner::Nextest), - Cargo::new("install") - .args("cargo-nextest --locked") - .name("Install nextest"), - ) + .add_step_when(matches!(self.test_runner, TestRunner::Nextest), { + let mut install = Cargo::new("install"); + install.args("cargo-nextest --locked"); + install.name("Install nextest"); + install + }) @@ - .add_step(match self.test_runner { - TestRunner::Cargo => Cargo::new("test") - .args("--all-features --workspace") - .name("Cargo Test"), - TestRunner::Nextest => Cargo::new("nextest") - .args("run --all-features --workspace") - .name("Cargo Nextest"), - }) + .add_step(match self.test_runner { + TestRunner::Cargo => { + let mut t = Cargo::new("test"); + t.args("--all-features --workspace"); + t.name("Cargo Test"); + t + } + TestRunner::Nextest => { + let mut n = Cargo::new("nextest"); + n.args("run --all-features --workspace"); + n.name("Cargo Nextest"); + n + } + }) @@ - .add_step_when( - self.benchmarks, - Cargo::new("bench").args("--workspace").name("Cargo Bench"), - ) + .add_step_when(self.benchmarks, { + let mut b = Cargo::new("bench"); + b.args("--workspace"); + b.name("Cargo Bench"); + b + })Also applies to: 179–185, 186–193, 204–212, 219–225, 226–229
🧹 Nitpick comments (1)
crates/gh-workflow-tailcall/src/standard.rs (1)
8-15: Minor docs nits: GitHub casing and a typo.
- Line 11: “Github” → “GitHub”.
- Line 33: “github action.” → “GitHub Action.”
- Line 110: “needs to by” → “needs to be”.
Apply this diff:
@@ -//! workflows you still need to constantly keep referring to the Github +//! workflows you still need to constantly keep referring to the GitHub @@ - /// *IMPORTANT:* Ensure `secrets.CARGO_REGISTRY_TOKEN` is set for your - /// github action. + /// *IMPORTANT:* Ensure `secrets.CARGO_REGISTRY_TOKEN` is set for your + /// GitHub Action. @@ - // NOTE: The workflow name needs to by `autofix.ci` + // NOTE: The workflow name needs to be `autofix.ci`Also applies to: 32-35, 110-111
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
README.md(1 hunks)crates/gh-workflow-tailcall/src/standard.rs(1 hunks)crates/gh-workflow/src/cargo.rs(1 hunks)crates/gh-workflow/src/event.rs(29 hunks)crates/gh-workflow/src/generate.rs(1 hunks)crates/gh-workflow/src/release_plz.rs(1 hunks)crates/gh-workflow/src/toolchain.rs(1 hunks)crates/gh-workflow/src/workflow.rs(14 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-10T06:33:21.046Z
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.
Applied to files:
crates/gh-workflow-tailcall/src/standard.rscrates/gh-workflow/src/workflow.rs
🪛 GitHub Actions: autofix.ci
crates/gh-workflow-tailcall/src/standard.rs
[error] 73-73: E0277: the trait bound &mut gh_workflow::Permissions: std::convert::Into<gh_workflow::Permissions> is not satisfied. Step: cargo +nightly clippy --fix --allow-dirty --all-features --workspace -- -D warnings.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and Test
🔇 Additional comments (6)
crates/gh-workflow/src/release_plz.rs (1)
8-8: Consistent setter pattern updateThe addition of
borrow_selfto theReleasestruct's setters aligns with the project-wide standardization of setter behavior. This ensures generated setters take&mut selfinstead of consumingself, enabling better method chaining patterns.crates/gh-workflow/src/generate.rs (1)
14-14: Consistent setter pattern updateThe addition of
borrow_selfto theGeneratestruct maintains consistency with the project-wide setter standardization. This change allows for more flexible method chaining by using mutable references instead of consuming the struct.crates/gh-workflow/src/event.rs (1)
14-14: Comprehensive setter pattern standardizationAll event-related structs now consistently use
borrow_selfin their setter derivation. This systematic change ensures uniform behavior across the entire event system, where all generated setters will take&mut selfinstead of consumingself. The change affects all major event configuration types includingEvent,BranchProtectionRule,CheckRun,CheckSuite,Create,Delete,Deployment,DeploymentStatus,Issues,Label,MergeGroup,Milestone,PullRequest,PullRequestReview,PullRequestReviewComment,PullRequestTarget,Push,RegistryPackage,Release,RepositoryDispatch,Schedule,Watch,WorkflowCall,WorkflowCallInput,WorkflowCallOutput,WorkflowCallSecret,WorkflowDispatch,WorkflowDispatchInput, andWorkflowRun.Also applies to: 97-97, 127-127, 153-153, 170-170, 197-197, 225-225, 244-244, 410-410, 441-441, 463-463, 487-487, 520-520, 563-563, 593-593, 611-611, 638-638, 685-685, 724-724, 740-740, 754-754, 768-768, 784-784, 799-799, 817-817, 829-829, 843-843, 852-852, 883-883
README.md (2)
38-38: Correct import path — LGTM.Good fix aligning the example with the published crate name.
37-49: All gh_workflows references removed
No remaining occurrences found in the repository.crates/gh-workflow-tailcall/src/standard.rs (1)
29-30: Setter receivers updated to borrow self — OK.Aligns with workspace-wide borrow_self migration for builder ergonomics.
| #[setters(strip_option, into, borrow_self)] | ||
| pub struct Cargo { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Switch to borrow_self alters setter receivers; ensure consistency with manual by-value builders.
With #[setters(..., borrow_self)], derived setters likely take a borrowed receiver (e.g., &mut self) for chaining, while methods like nightly, args, and add_args* still consume self. Mixed receiver styles can make chained usage awkward and may introduce breaking changes for prior one-liners that ended with a by-value method.
Recommendation:
- Decide on a consistent style. Either:
- Keep borrowed setters and add borrowed variants for manual methods, or
- Revert to by-value setters if preserving old chaining semantics is important.
- Document the new pattern in README (mutable binding + semicolon between chains, then call a by-value method).
Example aligning a couple of methods (illustrative):
- pub fn nightly(mut self) -> Self {
- self.toolchain = Some(Version::Nightly);
- self
- }
+ pub fn nightly(&mut self) -> &mut Self {
+ self.toolchain = Some(Version::Nightly);
+ self
+ }
- pub fn args<T: ToString>(mut self, args: T) -> Self {
- self.args = vec![args.to_string()];
- self
- }
+ pub fn args<T: ToString>(&mut self, args: T) -> &mut Self {
+ self.args = vec![args.to_string()];
+ self
+ }Run this quick check to spot potentially affected chaining patterns within the repo:
Also applies to: 38-49, 51-70
🏁 Script executed:
#!/usr/bin/env bash
rg -nP 'Cargo::new\([^)]*\)\.[[:alnum:]_]+\([^)]*\)\.([[:alnum:]_]+\()' -S -g '!**/target/**'Length of output: 220
Make manual builder methods borrow &mut self to match borrow_self setters
Manual methods (nightly, args, add_arg*) still consume and return self, breaking chained calls (e.g. line 228 in crates/gh-workflow-tailcall/src/standard.rs). Convert them to fn nightly(&mut self) -> &mut Self (and similarly for args, add_arg, add_args) to align with the borrow_self style, or revert the borrow_self option if you need to preserve the by-value chaining semantics.
🤖 Prompt for AI Agents
In crates/gh-workflow/src/cargo.rs around lines 7-8, the struct uses
#[setters(..., borrow_self)] but the manual builder methods (nightly, args,
add_arg, add_args) are defined to take and return self by value, breaking
chained calls; change their signatures to take &mut self and return &mut Self
and update their bodies so they mutate &mut self (not consume and re-create) and
end with &mut *self (or simply return self reference), ensuring they push/extend
fields directly on the mutable receiver; alternatively, if you prefer by-value
chaining semantics, remove the borrow_self option from the setters macro so
generated setters match the manual methods—make sure to update any call
sites/tests that assume the previous chaining style.
| #[setters(strip_option, into, borrow_self)] | ||
| pub struct Toolchain { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
borrow_self on Toolchain setters — OK, but update call sites.
This breaks expression-chaining into add_step/add_step_when where a Toolchain temporary is moved. Use local mut vars (see standard.rs suggestions) or add a finishing method if you prefer.
🏁 Script executed:
#!/bin/bash
# Find Toolchain builder chains likely to pass &mut into add_step(_when)
rg -n -C2 -P '\.add_step(?:_when)?\(\s*Toolchain::default\(\)[\s\S]*?\)'Length of output: 536
Update call sites broken by borrow_self setters
Changing to #[setters(..., borrow_self)] makes methods like add_stable take &mut self and return &mut Self, so chained calls such as
.add_step(Toolchain::default().add_stable())(e.g. crates/gh-workflow-tailcall/src/standard.rs:206) no longer compile. Refactor these to use a local mutable Toolchain (e.g.
let mut tc = Toolchain::default();
tc.add_stable();
job.add_step(tc);) or add a finalizer method that consumes the builder.
🤖 Prompt for AI Agents
In crates/gh-workflow/src/toolchain.rs around lines 143-144, changing the setter
attribute to include borrow_self makes builder methods take &mut self and return
&mut Self, which breaks call sites that expect an owned Toolchain from chained
expressions; update every call site (for example
crates/gh-workflow-tailcall/src/standard.rs:206) to instead create a local
mutable Toolchain, call the mutating setters on that local, then pass the final
Toolchain to the caller (i.e., let mut tc = Toolchain::default();
tc.add_stable(); job.add_step(tc);), or alternatively add a finalizer method on
Toolchain that consumes self and returns an owned Toolchain so chained
expression callers can use that consuming method to obtain an owned value.
| #[setters(strip_option, into, borrow_self)] | ||
| pub struct Workflow { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Broad adoption of borrow_self across public types — verify API break and update usage docs.
Applying borrow_self to setters for Workflow, Job, Container, Credentials, Volume, Concurrency, Permissions, Strategy, Environment, Defaults, RunDefaults, Secret, Artifacts, and Artifact shifts the chaining model. Prior chains like Workflow::default().name("X").generate() may no longer compile if setters return a borrowed receiver while generate(self) consumes by value.
Action items:
- Confirm this is an intentional semver-impacting change and call it out in CHANGELOG.
- Add a README snippet showing the new pattern:
- Use a mutable binding, chain borrowed setters, then call by-value methods in a separate statement.
- Example:
let mut wf = Workflow::new("CI"); wf.permissions(Permissions::default()).timeout_minutes(30); wf.generate()?;
- Consider parity: many manual builder methods still consume
self(e.g.,add_job,add_env, etc.). Decide whether to migrate them to borrowed receivers for a uniform API, or explicitly document the mix.
Quick repo scan to find potentially affected one-liners:
I can draft a short MIGRATION.md with before/after examples if helpful.
Also applies to: 223-224, 650-651, 684-685, 706-707, 733-734, 757-758, 813-814, 831-833, 844-846, 862-864, 896-898, 918-920, 932-934
🏁 Script executed:
#!/usr/bin/env bash
# Likely-breakers: one-line chains ending in a by-value method after a setter.
rg -nP '::(new|default)\([^)]*\)\.[[:alpha:]_]+\([^)]*\)\.generate\(' -S -g '!**/target/**'
rg -nP '\.name\([^)]*\)\.(to_string|parse|generate)\(' -S -g '!**/target/**'Length of output: 172
🏁 Script executed:
#!/usr/bin/env bash
# Search for generate usage chained after setters in the codebase
rg -nP '\.\w+\([^)]*\)\.generate\(' -S -g '!**/target/**'
# Look for any references to generate in documentation or examples
grep -R -n 'generate' README.md || true
grep -R -n 'generate' examples || true
grep -R -n 'generate' tests || trueLength of output: 507
Document semver-breaking setter change and update usage docs
- Add a breaking-change entry in CHANGELOG: setters annotated with
borrow_selfnow return&mut Self, so chains like
Workflow::default().name("X").generate()no longer compile. - Update README.md (around lines 35–52) to show the new pattern:
let mut wf = Workflow::new("CI"); wf.name("X").permissions(Permissions::default()); wf.generate().unwrap();
- Align or document the mixed builder API (e.g.
add_job,add_envstill consumingself) for consistency.
🤖 Prompt for AI Agents
In crates/gh-workflow/src/workflow.rs around lines 46-47, the addition of
#[setters(..., borrow_self)] made setters return &mut Self which is
semver-breaking; add a breaking-change entry to CHANGELOG.md describing that
setters now return &mut Self and that chained temporary-ownership usage (e.g.
Workflow::default().name("X").generate()) no longer compiles, update README.md
(around lines 35–52) to show the new usage pattern using a mutable binding (e.g.
let mut wf = Workflow::new("CI"); wf.name("X").permissions(...);
wf.generate().unwrap();), and either align the API so all builder methods
consistently borrow self or clearly document which methods consume self (e.g.
add_job/add_env) so callers know when to use mutable bindings versus
ownership-moving chains.
📝 Description
Fix incorrect import statement in the README usage example. The example was showing
use gh_workflows::*;when it should beuse gh_workflow::*;to match the actual crate name.🔧 Changes
gh_workflows::*togh_workflow::*🎯 Type of Change
✅ Impact
🧪 Testing
Summary by CodeRabbit