Skip to content

refactor: address copilot include diagnostics comments#5

Merged
satococoa merged 1 commit into
mainfrom
codex/copilot-followups
Mar 1, 2026
Merged

refactor: address copilot include diagnostics comments#5
satococoa merged 1 commit into
mainfrom
codex/copilot-followups

Conversation

@satococoa

@satococoa satococoa commented Mar 1, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Owner

Summary

  • standardize doctor report hint naming to IncludeMissingHint
  • export include origin/hint constants from engine and use them in cli
  • remove string literal coupling for include-origin and include-hint checks

Why

Copilot left 3 inline comments on the previous PR recommending:

  1. naming consistency between Result and DoctorReport
  2. avoiding magic-string coupling between engine and cli

This PR applies those recommendations directly.

Validation

  • go test ./...
  • make ci

Summary by CodeRabbit

  • Refactor
    • Standardized internal constants for include origin and hint values across the codebase.
    • Updated DoctorReport field naming for consistency (IncludeHint → IncludeMissingHint).
    • Replaced string literal comparisons with centralized constants for improved maintainability.

Copilot AI review requested due to automatic review settings March 1, 2026 17:07
@coderabbitai

coderabbitai Bot commented Mar 1, 2026 •
edited
Loading

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b7446f6 and 87e3eb4.

📒 Files selected for processing (3)
  • internal/cli/cli.go
  • internal/engine/engine.go
  • internal/engine/engine_integration_test.go
📝 Walkthrough

Walkthrough

The changes refactor include-related functionality by extracting hardcoded string literals into exported constants and renaming the IncludeHint field to IncludeMissingHint in the DoctorReport struct. Constants for include origins and missing hints are now centralized and exported from the engine package, ensuring consistency across the CLI and engine layers.

Changes

Cohort / File(s) Summary
Constants Externalization & Field Rename
internal/engine/engine.go		 Exports previously internal constants (IncludeOriginSource, IncludeOriginExplicit, IncludeMissingHintSourceMissing, IncludeMissingHintSourceMissingTargetExists) and renames DoctorReport.IncludeHint to IncludeMissingHint. Updates internal field assignments and comparisons to use the new exported names.
CLI Integration
internal/cli/cli.go		 Replaces hardcoded string literals in runApply, runDoctor, and formatIncludeStatusLine with the newly exported engine constants for origin labels and hint comparisons.
Test Updates
internal/engine/engine_integration_test.go		 Updates test assertions to reference IncludeMissingHint field and compare against IncludeMissingHintSourceMissingTargetExists constant instead of the previous IncludeHint field.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • Copilot

Poem

🐰 Constants hop from shade to light,
No more strings to hide from sight!
Field names shimmer, clean and bright,
Engine and CLI—oh what delight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main refactoring work: standardizing diagnostic-related naming and exporting constants to address Copilot review comments, which aligns with the substantial changes across engine and CLI packages.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/copilot-followups

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.

Copilot AI reviewed Mar 1, 2026
View reviewed changes

Copilot AI 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.

Pull request overview

This PR refactors include-diagnostics plumbing between internal/engine and internal/cli by standardizing the “include missing” hint naming and removing string-literal coupling via exported constants.

Changes:

  • Rename DoctorReport.IncludeHint to DoctorReport.IncludeMissingHint for consistency with Result.IncludeMissingHint.
  • Export include origin / missing-hint constants from internal/engine and use them from internal/cli.
  • Update integration tests and CLI formatting/checks to use the renamed field and exported constants.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
internal/engine/engine.go Renames doctor report field; exports include origin/missing-hint constants; updates internal assignments to use the exported constants.
internal/engine/engine_integration_test.go Updates assertions to use IncludeMissingHint and the exported hint constant.
internal/cli/cli.go Replaces magic-string comparisons with engine constants and switches doctor output to IncludeMissingHint.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@satococoa satococoa merged commit d3794df into main Mar 1, 2026
9 checks passed
@satococoa satococoa deleted the codex/copilot-followups branch March 1, 2026 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@satococoa