clojure-review

Clojure コードレビュースキル

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

レビューガイドライン

指摘すべきこと:

• Metabase Clojure スタイルガイド（上記に含まれる）への準拠を確認する
• 作業ディレクトリに CLOJURE_STYLE_GUIDE.adoc が存在する場合は、コミュニティの Clojure スタイルガイドへの準拠も確認する
• スタイルガイド違反をすべて指摘する

投稿すべきでないこと:

• 些細な変更やスタイルガイドの遵守に対して称賛するコメントを投稿しない
• 「問題なさそう」と確認したり、正しくできていると伝えるコメントを投稿しない
• スタイル違反や潜在的な問題についてのみコメントを投稿する

避けるべき悪いコードレビューコメントの例:

この TODO コメントは著者と日付が適切にフォーマットされていますね - よくできました！

クエリに limit 1 を追加したのは良いですね - これで動作を変えずにテストがより効率的になります。

kondo ignore コメントがここに適切に配置されています

テスト名がスタイルガイドの要求どおりに -test で正しく終わっています。

特殊なケース:

• 括弧の欠落についてのコメントは投稿しないでください（これらはリンターが検出します）

クイックレビューチェックリスト

変更を効率的に確認するためにこれを使用してください:

命名

• 説明的な名前（tbl、zs' を使わない）
• 純粋な関数は戻り値を表す名詞として命名されている
• すべての変数と関数に kebab-case を使用している
• 副作用のある関数は ! で終わっている
• 関数名に名前空間エイリアスの繰り返しがない

ドキュメント

• src または enterprise/backend/src のパブリック var に有用な docstring がある
• docstring は Markdown の慣習に従っている
• 参照にはバッククォートではなく [[other-var]] を使用している
• TODO コメントに著者と日付が含まれている: ;; TODO (Name 2025-01-01) -- description

docstring の内容 — 新規または書き直されたすべての docstring を、上記のスタイルガイドのアンチパターン表と照らし合わせて確認してください。diff の docstring は、コードとは別に単独で読んでください。散文はレビューの注意力が滑り落ちる場所であり、長い docstring は、それが通常そうであるところの負債としてではなく、勤勉さとして読まれてしまいます。

コード構成

• 他で使用されていない限りすべて ^:private
• 回避できる場合は declare を使わない（パブリック関数は最後の方に）
• 可能な場合は関数を 20 行未満にする
• 定義フォーム内に空の非コメント行を入れない（let/cond のペアワイズ構文を除く）
• 行は 120 文字以下

テスト

• 異なるテストケースごとに別々の deftest フォーム
• 純粋なテストには ^:parallel を付ける
• テスト名は -test または -test-<number> で終わる

モジュール

• 正しいモジュールパターン（OSS: metabase.<module>.*、EE: metabase-enterprise.<module>.*）
• API エンドポイントは <module>.api 名前空間にある
• パブリック API は Potemkin を使って <module>.core にある
• :clj-kondo/ignore [:metabase/modules] でモジュールリンターをごまかしていない

REST API

• レスポンススキーマが存在する（:- <schema>）
• クエリパラメータは kebab-case、ボディは snake_case を使用
• ルートは単数名詞を使用（例: /api/dashboard/:id）
• GET に副作用がない（アナリティクスを除く）
• Malli スキーマが詳細かつ完全である
• すべての新しいエンドポイントにテストがある

MBQL

• lib、lib-be、query-processor モジュール以外で生の MBQL を操作していない
• レガシー MBQL ではなく Lib と MBQL 5 を使用している

データベース

• モデル名とテーブル名は単数名詞
• 1 つのカラムのために全行を選択する代わりに t2/select-one-fn を使用している
• ロジックはヘルパー関数ではなく Toucan メソッドにある

ドライバー

• 新しいマルチメソッドが docs/developers-guide/driver-changelog.md に記載されている
• 他のドライバーメソッドに driver 引数を渡している（ドライバー名をハードコードしていない）
• read-column-thunk 内のロジックは最小限

その他

• 可能な場合はサンプルデータが鳥をテーマにしている
• Kondo リンターの抑制は適切な形式を使用している（#_:clj-kondo/ignore キーワード形式ではない）

パターンマッチング表

よくある問題をすばやく確認できます:

パターン	問題
calculate-age、get-user	純粋関数は名詞にするべきです: age、user
update-db、save-model	副作用を示す ! がありません: update-db!、save-model!
snake_case_var	kebab-case を使用するべきです
docstring のない公開 var	目的を説明する docstring を追加してください
;; TODO fix this	著者/日付がありません: ;; TODO (Name 2025-01-01) -- description
他の場所で使用されている名前空間内の (defn foo ...)	(defn ^:private foo ...) にするべきです
20 行を超える関数	より小さな関数に分割することを検討してください
/api/dashboards/:id	単数形を使用してください: /api/dashboard/:id
snake_case のクエリパラメータ	クエリパラメータには kebab-case を使用してください
テストのない新しい API エンドポイント	そのエンドポイントのテストを追加してください

フィードバック形式の例

スタイル違反の場合:

この純粋関数は、戻り値を表す名詞として命名するべきです。get-user の代わりに user を検討してください。

ドキュメントが欠落している場合:

この公開 var には、その目的、入力、出力を説明する docstring が必要です。

所有していないコードを記述している docstring の場合:

これは lib が内部で binning 戦略をどのように決定するかを説明しています。これが変更されてもここで失敗するものは何もないため、静かに陳腐化します。これを削除し、available-binning-strategies に自身の動作を説明させてください。

本体に属する docstring の内容の場合:

この段落は、なぜ分岐がこの順序になっているのかを説明しており、cond を編集する人に必要な情報です。しかし、呼び出し側はそれに基づいて行動できません。cond の上にあるインラインコメントに移動してください。

構成の問題の場合:

この関数はこの名前空間でのみ使用されているため、^:private とマークするべきです。

API 規約の場合:

クエリパラメータには kebab-case を使用するべきです。user_id を user-id に変更してください。