Skip to content

refactor(lib): combine_output を lib-subprocess に extract (順位 173a) - #205

Merged
aloekun merged 2 commits into
masterfrom
173a-combine-output-extract
Jun 13, 2026
Merged

refactor(lib): combine_output を lib-subprocess に extract (順位 173a)#205
aloekun merged 2 commits into
masterfrom
173a-combine-output-extract

Conversation

@aloekun

@aloekun aloekun commented Jun 13, 2026

Copy link
Copy Markdown
Owner

Summary

  • 5 crate (cli-pr-monitor / cli-push-runner / cli-push-pipeline / cli-merge-pipeline / hooks-post-tool-linter) に分散していた combine_output 8 行関数の重複実装を新規 lib-subprocess workspace crate に extract
  • 採用 variant は hooks-post-tool-linter 由来の \n suffix 吸収版 (production の callsite はすべて drain_pipe 経由で trailing newline を除去済の文字列を渡すため挙動差は顕在化しない、既存 4 crate の test も \n suffix case を含まないため全 case pass)
  • cli-pr-monitorcombine_output#[allow(dead_code)] の dead code だったため、dep 追加なしで関数 + duplicate test 削除のみ実施
  • 順位 173 (subprocess utils 統合) の挙動保存型 sub-PR 分割 (2026-06-14 ユーザー合意) の最初 (173a)。後続 173b (wait_with_timeout 2 variant)、173c (drain_pipe 3 variant)、173d (run_cmd)、173e (optional variant merge) は docs/todo11.md 内に温存

なぜこの分割か

  • 当初 todo 想定の「4 関数すべて 1 PR」は drain_pipe (MAX_LINES variants) / wait_with_timeout (Err 経路 kill 有無) の挙動 variant 統一を含み 500-800 行規模 + 挙動変更レビュー負荷大となる懸念
  • variant ごとに別 helper として export する「挙動保存型 staging」を採用、各 sub-PR を pure refactor として独立 land 可能に
  • 本 PR (173a) は pure 関数 1 個 / 5 crate touch / +162 -177 = net -15 行の小規模 refactor として完結

内訳

区分 内容
新規 src/lib-subprocess/ (Cargo.toml + lib.rs、combine_output(&str, &str) -> String + 5 variant test)
workspace ルート Cargo.toml に member 追加、Cargo.lock 更新
5 crate の Cargo.toml lib-subprocess dep 追加 (cli-pr-monitor を除く 4 crate)
5 crate の .rs 重複 impl 削除 + duplicate test 削除 + use lib_subprocess::combine_output; 追加
docs docs/todo11.md の 順位 173 entry を 173a/b/c/d/e に分割、docs/todo-summary.md の該当行を sub-PR 構造に更新

Test plan

  • cargo build --workspace pass
  • cargo test --workspace pass (lib-subprocess 5 test + 5 crate workspace 全 test)
  • cargo clippy --workspace -- -D warnings clean (push-runner-config.toml の gate と同等)
  • pre-push review APPROVED (1 iteration、F-1 / F-2 は non-blocking、いずれも lib.rs docstring に記述済 or pre-existing condition として整理)

注記

  • cargo clippy --workspace --all-targets -- -D warnings 時に cli-merge-pipeline/src/feedback.rs の事前 violation 2 件 (PR feat(lint): Bundle l (順位 129/131/132) — rule⑩ + diff metadata strip #159 vintage、items-after-test-module / assertions-on-constants) が露出するが、push pipeline の clippy gate は --all-targets を使わないため本 PR の gate 通過には影響しない。follow-up 候補
  • cli-merge-pipeline/src/main.rs (1003 行) と hooks-post-tool-linter/src/main.rs (3316 行) は touch-trigger ratchet で file_length lint の soft-feedback が発火するが、これは block しない nag のみで PR 全体のサイズに合った分割は別 task 扱い

関連

  • docs/todo11.md 順位 173 entry — sub-PR 分割計画の詳細
  • ADR-026 (Cargo workspace) / ADR-012 (lib-* naming convention) — 設計根拠
  • ADR-024 (lib-jj-helpers) — variant 別 helper export pattern の先例

Summary by CodeRabbit

  • Refactor

    • 複数モジュール間で重複していたサブプロセス処理ユーティリティを新規ライブラリに統合し、コード重複を削減。
  • Chores

    • ワークスペース構造を最適化し、保守性を向上。

@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a7ff4b81-66bb-4756-800a-d2a0ac0d2469

📥 Commits

Reviewing files that changed from the base of the PR and between 2a625c1 and 887e4d1.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (15)
  • Cargo.toml
  • docs/todo-summary.md
  • docs/todo10.md
  • docs/todo11.md
  • src/cli-merge-pipeline/Cargo.toml
  • src/cli-merge-pipeline/src/main.rs
  • src/cli-pr-monitor/src/runner.rs
  • src/cli-push-pipeline/Cargo.toml
  • src/cli-push-pipeline/src/main.rs
  • src/cli-push-runner/Cargo.toml
  • src/cli-push-runner/src/runner.rs
  • src/hooks-post-tool-linter/Cargo.toml
  • src/hooks-post-tool-linter/src/main.rs
  • src/lib-subprocess/Cargo.toml
  • src/lib-subprocess/src/lib.rs
💤 Files with no reviewable changes (1)
  • src/cli-pr-monitor/src/runner.rs

📝 Walkthrough

概要

新規の lib-subprocess ライブラリを作成して combine_output ヘルパー関数を実装し、5つのクレートの重複実装を置き換えるマイグレーション。ワークスペース追加、各クレートの依存更新、ローカル実装削除、および関連テスト調整を含む。

変更内容

lib-subprocess ライブラリ抽出と統合

Layer / File(s) Summary
lib-subprocess ライブラリの作成と combine_output 実装
Cargo.toml, src/lib-subprocess/Cargo.toml, src/lib-subprocess/src/lib.rs
新しい lib-subprocess クレートをワークスペースに追加し、stdout/stderr 結合ロジック(空値判定、末尾改行の有無による区切り文字制御)を実装。テストケース付き。
cli-merge-pipeline: lib-subprocess への移行
src/cli-merge-pipeline/Cargo.toml, src/cli-merge-pipeline/src/main.rs
combine_output のローカル関数定義を削除し lib_subprocess をインポート。付属テストも削除。
cli-push-pipeline: lib-subprocess への移行
src/cli-push-pipeline/Cargo.toml, src/cli-push-pipeline/src/main.rs
ローカル実装を削除して lib_subprocess::combine_output を参照。
cli-push-runner: lib-subprocess への移行とテスト更新
src/cli-push-runner/Cargo.toml, src/cli-push-runner/src/runner.rs
combine_output をローカルから lib_subprocess に置き換え。テストでは combine_output 単体検証を削除し run_stage_cmd と wait_with_timeout の動作確認に変更。
hooks-post-tool-linter: lib-subprocess への移行
src/hooks-post-tool-linter/Cargo.toml, src/hooks-post-tool-linter/src/main.rs
ローカル関数と関連テストを削除して lib_subprocess を参照。
cli-pr-monitor: ローカル実装の削除
src/cli-pr-monitor/src/runner.rs
combine_output 実装とテストを削除。本クレートへの統合は別途。
タスク追跡ドキュメントの更新
docs/todo-summary.md, docs/todo10.md, docs/todo11.md
タスク 173 の進捗を「173a: combine_output 完了」で更新。173b〜e の variant 抽出計画、および development-workflow.md と patterns.md への追記チェックリストを記録。

関連 PR

  • aloekun/claude-code-hook-test#27: cli-merge-pipeline の combine_output ローカル実装削除は PR #27 で導入された実装と同箇所を置き換える直接関連。

🎯 2 (Simple) | ⏱️ ~12 分

🚥 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 PR タイトルは、5つのクレートから重複していた combine_output 関数を lib-subprocess に抽出するという主要な変更を正確かつ簡潔に説明している。
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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 and usage tips.

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