chore(skills): common mistakes in lint rules#9972
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis PR adds “Common Mistakes to Avoid” documentation sections in two skill guides: Suggested labelsA-Linter Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.claude/skills/lint-rule-development/SKILL.md (1)
309-309: Soften theBox<str>guidance to avoid a false absolute.This currently reads as “
Box<str>inStateis a mistake”, but there are legitimate cases where owned text is needed. Suggest phrasing this as a preference, not a blanket rule.Suggested wording tweak
-- Placing `String` or `Box<str>` in a Rule's `State` type. It's a strong indicator that you are allocating a string unnecessarily. This can usually can be avoided by using `TokenText` instead. +- Placing `String` (and often `Box<str>`) in a Rule's `State` type when borrowed token text would do. This is often an unnecessary allocation; prefer `TokenText` where possible, and only use owned strings when ownership/lifetime requires it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/lint-rule-development/SKILL.md at line 309, Change the absolute guidance that “Placing `String` or `Box<str>` in a Rule's `State` type” is a mistake to a softer preference: state that using `String`/`Box<str>` in a `Rule`'s `State` may often be unnecessary and that `TokenText` is preferred when you can avoid allocating owned text, but acknowledge there are legitimate cases where owned text is required; update phrasing from categorical words like “is a strong indicator” to conditional language such as “often indicates” or “may indicate” and add a short clause that permits owned text when needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/lint-rule-development/SKILL.md:
- Around line 310-311: Fix the typo "interpolaion" to "interpolation" in the
sentence describing markup! interpolation and tighten the performance sentence
about run() vs action() for clarity and flow; specifically update the phrase
mentioning "run()" and "action()" so it reads more concise (e.g., "the run()
method should only determine whether to emit a diagnostic, and the action()
method should build the fix, because building the action can be expensive and
should be avoided when no diagnostic is emitted") while keeping references to
markup! interpolation and the run()/action() split.
---
Nitpick comments:
In @.claude/skills/lint-rule-development/SKILL.md:
- Line 309: Change the absolute guidance that “Placing `String` or `Box<str>` in
a Rule's `State` type” is a mistake to a softer preference: state that using
`String`/`Box<str>` in a `Rule`'s `State` may often be unnecessary and that
`TokenText` is preferred when you can avoid allocating owned text, but
acknowledge there are legitimate cases where owned text is required; update
phrasing from categorical words like “is a strong indicator” to conditional
language such as “often indicates” or “may indicate” and add a short clause that
permits owned text when needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0cdd19ec-c5c8-4a4b-b906-350838b8b697
📒 Files selected for processing (1)
.claude/skills/lint-rule-development/SKILL.md
d050d1f to
b239e4d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/lint-rule-development/SKILL.md:
- Line 309: Fix the duplicated word in the sentence about Rule state types: in
the line containing "Placing `String` or `Box<str>` in a Rule's `State` type..."
remove the extra "can" so it reads "This can usually be avoided by using
`TokenText` instead." Ensure the sentence still references `Rule`'s `State` and
`TokenText` to keep intent clear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0aca63b1-f194-48f2-9b8c-30a28e1e8d01
📒 Files selected for processing (1)
.claude/skills/lint-rule-development/SKILL.md
b239e4d to
d18457c
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.claude/skills/lint-rule-development/SKILL.md (1)
310-310: Great addition — tiny wording polish on Line 310.Mid-sentence,
E.g.reads a bit off in this style guide;e.g.,is the cleaner form in en‑GB prose.✏️ Suggested tweak
-- - Calling `format!()` (allocates a string) when formatting strings in a `markup!` block. `markup!` supports interpolation, E.g. `markup! { "Hello, "{name}"!" }`. +- Calling `format!()` (allocates a string) when formatting strings in a `markup!` block. `markup!` supports interpolation, e.g., `markup! { "Hello, "{name}"!" }`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/lint-rule-development/SKILL.md at line 310, Update the wording at the sentence that begins "Calling `format!()` (allocates a string) when formatting strings in a `markup!` block. `markup!` supports interpolation, E.g. `markup! { "Hello, "{name}"!" }`" by replacing "E.g." with the lowercase "e.g.," so it reads "...supports interpolation, e.g., `markup! { "Hello, "{name}"!" }`" to match en‑GB punctuation and style; locate the exact sentence in the SKILL.md content near the `format!()`/`markup!` example and apply this single-word, punctuation change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.claude/skills/lint-rule-development/SKILL.md:
- Line 310: Update the wording at the sentence that begins "Calling `format!()`
(allocates a string) when formatting strings in a `markup!` block. `markup!`
supports interpolation, E.g. `markup! { "Hello, "{name}"!" }`" by replacing
"E.g." with the lowercase "e.g.," so it reads "...supports interpolation, e.g.,
`markup! { "Hello, "{name}"!" }`" to match en‑GB punctuation and style; locate
the exact sentence in the SKILL.md content near the `format!()`/`markup!`
example and apply this single-word, punctuation change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a317e405-5b03-47de-a72e-7e4450980ccf
📒 Files selected for processing (1)
.claude/skills/lint-rule-development/SKILL.md
d18457c to
c92430e
Compare
Summary
tired of fixing/giving feedback on the same things over and over
Test Plan
Docs