Skip to content

chore(push-runner): quality_gate clippy を --all-targets --all-features 化 (順位259) - #251

Merged
aloekun merged 1 commit into
masterfrom
quality-gate-clippy-all-targets
Jul 6, 2026
Merged

chore(push-runner): quality_gate clippy を --all-targets --all-features 化 (順位259)#251
aloekun merged 1 commit into
masterfrom
quality-gate-clippy-all-targets

Conversation

@aloekun

@aloekun aloekun commented Jul 6, 2026

Copy link
Copy Markdown
Owner

概要

push pipeline の quality_gate clippy を cargo clippy --workspace -- -D warnings から --all-targets --all-features 化し、#[cfg(test)] ユニットテスト・integration test のコードも lint 対象に含める。順位259(PR #247 post-merge-feedback T2-1 採用)。

動機

現行 clippy は lib/bin ターゲットのみを検査し、テストコードが lint されない gap があった。PR #247useless_format が clippy を素通りして cargo test 段階まで顕在化し手戻りが発生した。--all-targets(= --lib --bins --tests --benches --examples)でテストコードも clippy 対象になる。

変更

  • push-runner-config.toml + templates/push-runner-config.tomlrust-lint-test group clippy を --all-targets --all-features
  • 既存違反 1 件(cli-merge-pipelinefeedback/takt.rsORPHAN_THRESHOLD_SECS 不変条件を検証する runtime test の clippy::assertions_on_constants)を const _: () = assert!(...) のコンパイル時検証へ移行(定数条件は test 実行時 assert より compile-time assert の方が強い保証。runtime test は除去)
  • ADR-015 § clippy の lint スコープ を amendment

検証

備考

  • --all-features は現時点で全 crate に [features] 定義がなく no-op(将来の feature-gated コード用の保険)
  • 実測コスト(2026-07-05): clippy ウォームで +1〜3s、rust-lint-test group は cargo test -- --ignored(~80s)支配で誤差、4 group 並列のため総時間影響なし
  • todo 順位259 を消化(todo13.md エントリ + todo-summary.md 行)

Summary by CodeRabbit

  • Documentation

    • 品質チェックに関するガイドと実行手順を更新し、lint 対象がテストコードや全機能にも広がることを明記しました。
    • 実行例の記載も最新の設定に合わせて整理しました。
  • Bug Fixes

    • 主要な lint 実行範囲を拡張し、これまで見逃される可能性があったテスト関連コードの警告も検出されるようになりました。
    • 重要な閾値の整合性を、実行時ではなくビルド時に確認するよう改善しました。

…s 化 (順位259)

現行 `cargo clippy --workspace -- -D warnings` は lib/bin ターゲットのみを検査し、
#[cfg(test)] ユニットテスト・integration test のコードが lint されない gap があった。
PR #247 で useless_format が cargo test 段階まで顕在化して手戻りが発生したことを受け、
rust-lint-test group の clippy を --all-targets --all-features 化する。

- push-runner-config.toml + templates/push-runner-config.toml を同スコープに更新
- 既存違反 1 件 (cli-merge-pipeline takt.rs の ORPHAN_THRESHOLD_SECS 不変条件 runtime
  test の clippy::assertions_on_constants) を const _: () = assert!(...) のコンパイル時
  検証へ移行 (定数条件は test 実行時 assert より compile-time assert の方が強い保証)
- cargo clippy --workspace --all-targets --all-features -- -D warnings が workspace 全体 PASS
- ADR-015 § clippy の lint スコープ を amendment。todo 順位259 を消化。
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

quality_gate の clippy 実行コマンドに --all-targets --all-features を追加し、ADR-015 に方針を追記。takt.rs の閾値検証をランタイムテストからコンパイル時の const assert に移行。関連する TODO ドキュメントのタスク項目を削除・追加した。

Changes

quality_gate clippy スコープ拡張と関連整理

Layer / File(s) Summary
clippy コマンドの拡張
push-runner-config.toml, templates/push-runner-config.toml
cargo clippy 実行に --workspace --all-targets --all-features を付与し、テンプレート側のコメント記述も同様に更新。
ADR-015 の方針追記
docs/adr/adr-015-push-runner-takt-migration.md
clippy のlint対象ギャップ、コスト見込み、compile-time検証への移行、テンプレート同期方針を追記。
takt.rs の compile-time 検証移行
src/cli-merge-pipeline/src/feedback/takt.rs
ORPHAN_THRESHOLD_SECSTAKT_TIMEOUT_SECS の関係検証を const assert に置き換え、対応するテストを削除。
関連 TODO の整理
docs/todo-summary.md, docs/todo13.md
clippy スコープ拡張タスク(PR #247)を削除し、todo13.md に別タスク(PR #245)を追加。

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed quality_gate の clippy を --all-targets --all-features 化する主変更を的確に表しており、変更内容と一致しています。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch quality-gate-clippy-all-targets

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@aloekun

aloekun commented Jul 6, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/cli-merge-pipeline/src/feedback/takt.rs (1)

33-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

現状の定義では両アサーションは自明(トートロジー)です。

ORPHAN_THRESHOLD_SECS は Line 31 で TAKT_TIMEOUT_SECS + 300 として定義されているため、両方の const assert! は現状のコードでは常に真になります。将来どちらかの定数がハードコード値に変更された際の回帰防止としては有用なので問題視はしませんが、その意図であればコメントで明記しておくとレビュー時の混乱を防げます。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli-merge-pipeline/src/feedback/takt.rs` around lines 33 - 40, The two
compile-time assertions in takt.rs are currently tautological because
ORPHAN_THRESHOLD_SECS is defined directly from TAKT_TIMEOUT_SECS, so the intent
needs to be made explicit. Update the nearby definition/comments around
ORPHAN_THRESHOLD_SECS and the const assert! checks in takt.rs to state that
these assertions are deliberate regression guards against future hardcoded
changes to TAKT_TIMEOUT_SECS or the orphan threshold, using the symbols
ORPHAN_THRESHOLD_SECS and TAKT_TIMEOUT_SECS to make that purpose clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/cli-merge-pipeline/src/feedback/takt.rs`:
- Around line 33-40: The two compile-time assertions in takt.rs are currently
tautological because ORPHAN_THRESHOLD_SECS is defined directly from
TAKT_TIMEOUT_SECS, so the intent needs to be made explicit. Update the nearby
definition/comments around ORPHAN_THRESHOLD_SECS and the const assert! checks in
takt.rs to state that these assertions are deliberate regression guards against
future hardcoded changes to TAKT_TIMEOUT_SECS or the orphan threshold, using the
symbols ORPHAN_THRESHOLD_SECS and TAKT_TIMEOUT_SECS to make that purpose clear.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f42af638-0776-49a8-981d-99a6ffdc0932

📥 Commits

Reviewing files that changed from the base of the PR and between eb992a8 and ecdb865.

📒 Files selected for processing (6)
  • docs/adr/adr-015-push-runner-takt-migration.md
  • docs/todo-summary.md
  • docs/todo13.md
  • push-runner-config.toml
  • src/cli-merge-pipeline/src/feedback/takt.rs
  • templates/push-runner-config.toml
💤 Files with no reviewable changes (2)
  • docs/todo-summary.md
  • docs/todo13.md

@aloekun
aloekun merged commit 4a0f3f3 into master Jul 6, 2026
1 check passed
@aloekun
aloekun deleted the quality-gate-clippy-all-targets branch July 6, 2026 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant