clojure-review

SKILL.md

Clojure Code Review Skill

@./../_shared/clojure-style-guide.md
@./../_shared/clojure-commands.md

Review guidelines

What to flag:

Check compliance with the Metabase Clojure style guide (included above)

If CLOJURE_STYLE_GUIDE.adoc exists in the working directory, also check compliance with the community Clojure style guide

Flag all style guide violations

What NOT to post:

Do not post comments congratulating someone for trivial changes or for following style guidelines

Do not post comments confirming things "look good" or telling them they did something correctly

Only post comments about style violations or potential issues

Example bad code review comments to avoid:

This TODO comment is properly formatted with author and date - nice work!

Good addition of limit 1 to the query - this makes the test more efficient without changing its behavior.

The kondo ignore comment is appropriately placed here

Test name properly ends with -test as required by the style guide.

Special cases:

Do not post comments about missing parentheses (these will be caught by the linter)

Quick review checklist

Use this to scan through changes efficiently:

Naming

 Descriptive names (no tbl, zs')

 Pure functions named as nouns describing their return value

 kebab-case for all variables and functions

 Side-effect functions end with !

 No namespace-alias repetition in function names

Documentation

 Public vars in src or enterprise/backend/src have useful docstrings

 Docstrings use Markdown conventions

 References use [[other-var]] not backticks

 TODO comments include author and date: ;; TODO (Name 2025-01-01) -- description

Code Organization

 Everything ^:private unless used elsewhere

 No declare when avoidable (public functions near end)

 Functions under 20 lines when possible

 No blank, non-comment lines within definition forms (except pairwise constructs in let/cond)

 Lines ≤ 120 characters

Tests

 Separate deftest forms for distinct test cases

 Pure tests marked ^:parallel

 Test names end in -test or -test-<number>

Modules

 Correct module patterns (OSS: metabase.<module>.*, EE: metabase-enterprise.<module>.*)

 API endpoints in <module>.api namespaces

 Public API in <module>.core with Potemkin

 No cheating module linters with :clj-kondo/ignore [:metabase/modules]

REST API

 Response schemas present (:- <schema>)

 Query params use kebab-case, bodies use snake_case

 Routes use singular nouns (e.g., /api/dashboard/:id)

 GET has no side effects (except analytics)

 Malli schemas detailed and complete

 All new endpoints have tests

MBQL

 No raw MBQL manipulation outside lib, lib-be, or query-processor modules

 Uses Lib and MBQL 5, not legacy MBQL

Database

 Model and table names are singular nouns

 Uses t2/select-one-fn instead of selecting full rows for one column

 Logic in Toucan methods, not helper functions

Drivers

 New multimethods documented in docs/developers-guide/driver-changelog.md

 Passes driver argument to other driver methods (no hardcoded driver names)

 Minimal logic in read-column-thunk

Miscellaneous

 Example data is bird-themed when possible

 Kondo linter suppressions use proper format (not #_:clj-kondo/ignore keyword form)

Pattern matching table

Quick scan for common issues:

Pattern
Issue

calculate-age, get-user
Pure functions should be nouns: age, user

update-db, save-model
Missing ! for side effects: update-db!, save-model!

snake_case_var
Should use kebab-case

Public var without docstring
Add docstring explaining purpose

;; TODO fix this
Missing author/date: ;; TODO (Name 2025-01-01) -- description

(defn foo ...) in namespace used elsewhere
Should be (defn ^:private foo ...)

Function > 20 lines
Consider breaking up into smaller functions

/api/dashboards/:id
Use singular: /api/dashboard/:id

Query params with snake_case
Use kebab-case for query params

New API endpoint without tests
Add tests for the endpoint

Feedback format examples

For style violations:

This pure function should be named as a noun describing its return value. Consider user instead of get-user.

For missing documentation:

This public var needs a docstring explaining its purpose, inputs, and outputs.

For organization issues:

This function is only used in this namespace, so it should be marked ^:private.

For API conventions:

Query parameters should use kebab-case. Change user_id to user-id.

1d