agentsclimarketplace

Code review guidelines

Skill KraitDev/skiLL.Md/skills/code-quality/code-review-guidelines

skiLL.Md is a structured, open-source collection of reusable, self-contained markdown modules that describe how to perform specific software engineering tasks.

Install
npx -y skills add KraitDev/skiLL.Md --skill code-review-guidelines

Assembled from the repository path, not quoted from the project. Check it against their README if it does not work.

One thing to look at

  • 6 stars6 stars. Stars are a popularity signal and not a quality one, but at this level it is likely that nobody has read this closely except its author, and you would be relying on your own review.

What its author says it does

Copied from the file, not written here

When asynchronously reviewing peer code before merging into the main branch.

The file declares its own license as MIT. That is the author’s claim about this one file, and it is not the same thing as the license GitHub reports for the repository, which is listed with the other numbers below.

SKILL.md

6.0 KB, as published. Nobody here has run it

Code Review Guidelines

Purpose

Code review catches bugs, ensures maintainability, and builds team knowledge. This skill provides a structured, objective, empathetic framework for reviewing code without causing friction. The goal is to improve the codebase while growing the team.

When to use

  • Evaluating a Pull Request or Merge Request
  • Providing constructive feedback to peers
  • Ensuring code quality and architectural standards are met
  • Building new team members' skills through feedback

When NOT to use

  • Nitpicking personal coding style (use linters/formatters)
  • Blocking on subjective preferences
  • Reviewing without understanding context

Inputs required

  • Pull request with code changes
  • PR description explaining the why
  • Linked ticket with context
  • Test coverage (unit/integration tests)

Workflow

  1. Understand the Goal: Read the PR description and linked ticket FIRST. Understand WHY the change exists before reading code.
  2. Review Architecture: Check structural integrity and design. Does it fit the codebase? Are new dependencies justified? Is it the right abstraction level?
  3. Review Logic: Check for correct error handling, edge cases, off-by-one errors, and security implications.
  4. Check Tests: Verify new logic is covered by unit/integration tests that test actual behavior (not just code coverage).
  5. Consider Performance: Check for obvious performance issues (N+1 queries, memory leaks, synchronous blocking in async code).
  6. Provide Feedback: Leave comments that explain the WHY. Differentiate between blocking requests and suggestions (use "Nit:" or "Note:").
  7. Be Constructive: Phrase feedback as questions when possible ("Have you considered...?"). Assume positive intent.

Rules

  • MUST assume positive intent; critique the code, not the author
  • MUST automate style and formatting checks via Linters/Formatters (do NOT argue over spacing in PR)
  • MUST approve immediately if code improves the codebase, even if not perfect
  • MUST provide context and links when referring to standards or patterns
  • MUST distinguish between blocking requests and optional suggestions
  • MUST NOT block on personal coding style preferences
  • MUST NOT require unrelated changes (keep scope focused)

Anti-patterns

  • Gatekeeping: Blocking PRs over personal style preferences or "I would have done it differently"
  • Rubber Stamping: Approving large PRs without reading or thinking
  • Scope Creep: Asking the author to fix unrelated legacy code nearby
  • Vague Comments: "This is bad" or "Improve this" without explanation
  • Late Requests: Requesting major architectural changes after code is already implemented
  • Tone Issues: Comments that feel dismissive or condescending

Failure conditions

  • PR blocked without clear explanation
  • Security or correctness issue missed
  • No tests for new logic
  • Feedback based on personal preference, not code quality
  • Author feels disrespected or attacked

Validation checklist

  • PR description explains the why (not just what)
  • Tests cover new logic
  • No obvious performance issues
  • Error handling is correct
  • Edge cases are considered
  • Security implications reviewed (if applicable)
  • Code follows established patterns in the codebase
  • Dependencies are justified
  • Comments explain complex logic
  • No scope creep (focused on this change)

Output format

  • Feedback structure: Problem + context + suggestion
  • Tone: Constructive, curious, respectful
  • Clarity: Blocking requests vs. optional suggestions clearly marked
  • Actionability: Comments are specific and actionable
  • Decision: Clear approval, changes requested, or comment

Security considerations

  • Security issues MUST be flagged as blocking
  • SQL injection, XSS, authentication bypasses MUST be caught
  • Secrets/credentials MUST NEVER be committed (catch before merge)
  • Permission checks MUST be enforced
  • Data exposure MUST be reviewed

Agent execution notes

  • Agent MAY: Leave constructive feedback, ask clarifying questions, suggest improvements
  • Agent MUST NEVER: Block on style/preference, be dismissive, miss security issues, approve without reading
  • Agent MUST ASK: When context is missing, when architectural impact is unclear
  • Agent MUST VALIDATE: Tests are present, no security holes, code improves codebase

Example

❌ Anti-pattern (Vague, dismissive, scope creep):

"This code is bad"
"Why are you doing it this way?"
"Also, can you fix the bug in the file next to this?"
"Blocked - needs improvement"
"LGTM" (without reading)

✅ Correct pattern (Constructive, specific, respectful):

✅ Good catch on the edge case - this prevents the race condition we had in production.

💭 Question: In the validateEmail function, what happens if the email service is down?
   Have you considered adding a timeout or fallback behavior?

🔒 Security: Double-check that user input is sanitized before the database query.

📝 Nit: This comment could be clearer - what does "process the thing" mean?

✅ The test coverage looks good. I especially like the edge case tests.

🚀 Approved! This is a clear improvement to the codebase. Nice work.

Keep looking

Skills are one crate of 328,083. Ordering is by how many stacks a row turns up in, so the top of any crate is what has actually been picked rather than what has the most stars.