docs(code-review): add integrations section pointing to open-code-review - #327
docs(code-review): add integrations section pointing to open-code-review#327lizhengfeng101 wants to merge 3 commits into
Conversation
|
Thanks for this, the skill itself is well-crafted and the underlying tool looks genuinely solid. Before it goes further I want to raise a scope question that I think is the real decision here, and it is ultimately a maintainer call. My concern is fit, not quality. The skills in this pack encode a process, a tool-agnostic way a senior engineer approaches a task. This skill is a different genre: it teaches the agent to operate one specific vendor CLI and format its output. The review intelligence lives inside Three things make me lean against adding it as a first-class skill:
The browser-testing-with-devtools precedent does not quite apply: DevTools gives the agent a capability it otherwise lacks (observing a running browser), whereas this is a different vendor's take on a job an existing skill already covers. That said, the insight behind the tool is worth keeping. The failure mode it targets is real: prompt-only agents do skip files on large changesets and drift on line numbers. A lighter-touch way to capture that without taking on a vendor dependency would be a short "See also / Integrations" note in (Separately and minor: the |
|
Thank you for the thoughtful and detailed feedback, @nucliweb — really appreciate you taking the time to lay out the reasoning so clearly. We want to sincerely apologize if our description came across as positioning We fully accept the suggested alternative approach. A short "See also / Integrations" note in We'll close this PR accordingly. If there's anything we can do to help shape that integrations note, happy to contribute. |
Add a new skill that integrates alibaba/open-code-review (ocr) CLI into the agent-skills workflow. This complements the existing code-review-and-quality skill with a tool-augmented approach that combines deterministic engineering with an LLM agent for higher precision at ~1/9 token cost.
Instead of adding a standalone skill, add a lightweight "Integrations" note under See Also in code-review-and-quality, pointing users to the ocr CLI and its Claude Code skill for tool-augmented review on large changesets.
The tool is now referenced as an integration in code-review-and-quality instead of being a separate skill, per reviewer feedback on #327.
99f3287 to
cc30f17
Compare
|
Hi @nucliweb 👋 Thanks again for the excellent feedback on the original approach — it was spot on. We've reworked this PR to follow the alternative you suggested: a short Integrations section in We also rebased onto the latest Would you be able to take another look when you get a chance? Happy to adjust anything if the wording or placement needs tweaking. Thanks! |
|
This is softer than the original standalone-skill version, and I get the intent of pointing people at tools that complement the review workflow. But it's still a link out to a specific third-party product from inside a core skill, which is the kind of mention I've been keeping the pack clear of, and once one integrations pointer goes in others follow. I'd rather not open that door in the skill files themselves. Leaning pass, but tell me if you think this one clears a bar the rest don't. |
|
I share the pass instinct: keeping skill files free of product-specific links is the only rule that doesn't invite a queue of similar PRs, and any exception becomes the precedent the next one cites. One factual note from checking the diff: the "Claude Code skill" link points to anthropics/claude-code-installed-skills, which currently 404s. A live example of the link-rot risk in hosting pointers to surfaces we don't control. @nucliweb you shaped this version, so I'd like your take before we decide. |
|
Hi everyone 👋 Thanks for all the thoughtful feedback on this PR — the concerns about scope, vendor coupling, and the "integrations pointer" precedent were well taken. We've taken a different approach in #408 that addresses the core issues raised here:
This PR can be closed in favor of #408. Happy to adjust anything there based on further feedback. |
|
@nucliweb @addyosmani @federicobartoli Would appreciate your review on #408 when you get a chance. Thanks! |
|
Thanks for the graceful way you handled the feedback here, and for reworking the approach into #408. Closing this one as you suggested, since it's superseded by #408 and the integrations-pointer route was the one Addy was leaning to pass on anyway. The scope and vendor-coupling discussion in this thread stays useful as context for reviewing #408, so we'll pick it up from there. |
Summary
Instead of adding a standalone skill, this adds a lightweight "Integrations" subsection under "See Also" in
code-review-and-quality/SKILL.md, pointing users to the Open Code Review CLI and its Claude Code skill for tool-augmented review on large changesets.This follows the alternative approach suggested by @nucliweb in #327 (comment) — keeping the pack self-contained and zero-dependency while still letting users discover additional tooling.
Changes
skills/code-review-and-quality/SKILL.mdskills/open-code-review/directoryREADME.md,AGENTS.md, anddocs/getting-started.md