Skip to content

Enforce non-empty SmeeConfig hook phases - #196

Merged
errfld merged 1 commit into
mainfrom
gh-195/non-empty-config-hooks
Jul 27, 2026
Merged

Enforce non-empty SmeeConfig hook phases#196
errfld merged 1 commit into
mainfrom
gh-195/non-empty-config-hooks

Conversation

@errfld

@errfld errfld commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • make SmeeConfig hook storage private and add validated construction/read APIs
  • reject explicitly empty lifecycle phases during both Rust construction and TOML deserialization
  • migrate executor, installer, CLI diagnostics, tests, and public examples away from direct map access

TDD

  • RED: cargo test -p git-smee-core --test config_integration failed because try_new and hooks_for did not exist
  • GREEN: focused config integration suite passed with construction, deserialization, round-trip, and lookup regressions

Validation

  • cargo +1.92.0 fmt --all -- --check
  • cargo +1.92.0 clippy --workspace --all-targets --all-features --locked -- -D warnings
  • CARGO_BUILD_JOBS=1 CARGO_INCREMENTAL=0 cargo +1.92.0 test --workspace --all-targets --all-features --locked (238 tests passed)

The first full-suite attempt encountered a linker bus error after the disk reached 100%; cleaning this worktree's rebuildable target/ artifacts and rerunning serially produced the full green result above.

Fixes #195

Summary by CodeRabbit

  • New Features

    • Added structured configuration APIs for inspecting configured lifecycle phases and hooks.
    • Configuration now rejects lifecycle phases that contain no hooks, with clearer validation errors.
  • Bug Fixes

    • Updated diagnostics, status reporting, installation, and hook execution to consistently recognize configured phases and hook counts.
    • Improved configuration loading and round-trip validation for multi-phase setups.

@coderabbitai

coderabbitai Bot commented Jul 27, 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: ASSERTIVE

Plan: Pro Plus

Run ID: ab62c1df-cec0-4b7a-8f47-7f37bf4556e9

📥 Commits

Reviewing files that changed from the base of the PR and between a7a8485 and f223f03.

📒 Files selected for processing (8)
  • crates/git-smee-cli/src/doctor.rs
  • crates/git-smee-cli/src/status.rs
  • crates/git-smee-cli/tests/cli_integration.rs
  • crates/git-smee-core/src/config.rs
  • crates/git-smee-core/src/executor.rs
  • crates/git-smee-core/src/installer.rs
  • crates/git-smee-core/tests/config_integration.rs
  • crates/git-smee-core/tests/installer_integration.rs

📝 Walkthrough

Walkthrough

SmeeConfig now enforces non-empty hook phases through try_new, hides its storage map, and provides query APIs. Core execution, installation, and CLI reporting use those APIs, while unit and integration tests cover validation, deserialization, round-tripping, and migrated construction.

Changes

Validated configuration API

Layer / File(s) Summary
Validated SmeeConfig contract
crates/git-smee-core/src/config.rs, crates/git-smee-core/tests/config_integration.rs
The hook map is private; try_new, hooks_for, phases, phase_count, and is_empty provide validated construction and read-only inspection. Empty phases are rejected during construction and deserialization.
Core and CLI consumer migration
crates/git-smee-core/src/executor.rs, crates/git-smee-core/src/installer.rs, crates/git-smee-cli/src/doctor.rs, crates/git-smee-cli/src/status.rs, crates/git-smee-cli/tests/cli_integration.rs
Execution, installation, doctor reports, status reports, and CLI parsing assertions use the phases-oriented APIs instead of direct map access.
Validated construction test migration
crates/git-smee-core/src/executor.rs, crates/git-smee-core/src/installer.rs, crates/git-smee-core/tests/installer_integration.rs
Fixtures and examples construct configurations through SmeeConfig::try_new while retaining existing execution and installation coverage.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Poem

A bunny found hooks in a neat little row,
With phases that never hold zero below.
The map hid away, APIs led the way,
Reports now hop through the proper display.
“Validated!” I wiggle my nose—
And off through the clean config goes!

🚥 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 The title clearly matches the main change: enforcing non-empty hook phases in SmeeConfig.
Linked Issues check ✅ Passed The PR implements private storage, validated construction, read-only APIs, TOML validation, and consumer migrations required by #195.
Out of Scope Changes check ✅ Passed The changes stay focused on SmeeConfig validation, accessors, and related migrations without introducing unrelated scope.
Docstring Coverage ✅ Passed Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%.
✨ 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 gh-195/non-empty-config-hooks

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.

@errfld
errfld merged commit d06e8aa into main Jul 27, 2026
28 checks passed
@errfld
errfld deleted the gh-195/non-empty-config-hooks branch July 27, 2026 06:36
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.

Make SmeeConfig enforce non-empty hook lists

2 participants