Skip to content

refactor: deduplicate optional variable checks - #797

Merged
marcusburghardt merged 1 commit into
complytime:mainfrom
tomatotomata:codex/group-satisfied-refactor
Sep 15, 2026
Merged

marcusburghardt merged 1 commit into
complytime:mainfrom
tomatotomata:codex/group-satisfied-refactor

Conversation

@tomatotomata

Copy link
Copy Markdown
Contributor

Summary

Extract the duplicated pipe-splitting lookup used for optional target variable groups in CheckVariables into a small groupSatisfied helper. The summary and verbose paths now share the same implementation, preserving the existing behavior while reducing maintenance surface.

Closes #779

Validation

  • go test -count=1 ./internal/doctor
  • go vet ./internal/doctor
  • gofmt -w internal/doctor/doctor.go
  • git diff --check
  • Divisor council review: Architect, Tester, and Guard all approved.

The repository's full go test -race command could not run locally because this Windows environment has no C compiler for cgo. The change is a pure refactor and the targeted package tests pass.

I used AI assistance for this small refactor and reviewed the repository's AI tooling and contribution guidance.

@marcusburghardt

Copy link
Copy Markdown
Member

Hi @tomatotomata , thanks for the contribution. What are your plans to continue on this PR? After rebasing it, the failed CI will likely pass.

@tomatotomata

Copy link
Copy Markdown
Contributor Author

I rebased the one-file refactor locally onto current upstream main (6abc690) as ad8e148, without conflicts.

The focused checks on the rebased tree pass:

  • go test -count=1 ./internal/doctor
  • go vet ./internal/doctor
  • gofmt -d internal/doctor/doctor.go
  • git diff --check

The implementation is unchanged apart from the rebased parent, so the failed CI should be worth rerunning against this history. Let me know what you think about the rebased result.

@marcusburghardt
marcusburghardt force-pushed the codex/group-satisfied-refactor branch from dd7815c to 1241f66 Compare September 11, 2026 10:52
@marcusburghardt
marcusburghardt marked this pull request as ready for review September 11, 2026 10:52
@marcusburghardt
marcusburghardt requested a review from a team as a code owner September 11, 2026 10:52

@marcusburghardt marcusburghardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@marcusburghardt marcusburghardt added the enhancement New feature or request label Sep 11, 2026
@marcusburghardt
marcusburghardt force-pushed the codex/group-satisfied-refactor branch from 1241f66 to b8b7bee Compare September 15, 2026 08:17

@gxmiranda gxmiranda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Clean, well-scoped refactoring that extracts the duplicated pipe-split-and-lookup pattern from two call sites in CheckVariables() into a shared groupSatisfied helper. Net -3 lines, single file, behavior-preserving. All CI checks pass. Aligns with issue #779 and Constitution Principle I (DRY).

This review was generated by /review-pr (AI-assisted).

@marcusburghardt
marcusburghardt merged commit 94c1f79 into complytime:main Sep 15, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(doctor): extract groupSatisfied helper to deduplicate pipe-split logic in CheckVariables

4 participants