Skip to content

Correct the exit-code and Docker claims in OPERATIONS.md - #46

Merged
ptr727 merged 1 commit into
developfrom
operations-exit-codes-docker
Aug 3, 2026
Merged

Correct the exit-code and Docker claims in OPERATIONS.md#46
ptr727 merged 1 commit into
developfrom
operations-exit-codes-docker

Conversation

@ptr727

@ptr727 ptr727 commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Answers both Copilot findings raised on the develop to main promotion PR #45. Both are correct.

They only surfaced there because the promotion diffs against main, so OPERATIONS.md is read as a whole file alongside the verify command from #35. Reviewed against develop, the file is unchanged and neither claim is in the diff. Two statements were written true and were left behind when verify landed.

1. Exit codes

OPERATIONS.md:68 - This exit-code documentation is now stale. Commands have been updated to use the shared ExitCode contract (0 success, 1 could not run, 2 completed with failures), so per-file failures are no longer reported as exit 0.

Correct. PhotoCleaner/ExitCode.cs defines Failed = 2, and seven call sites return it: TrashCommand.cs:163, IndexCommand.cs:42, ImportCommand.cs:88, ProcessCommand.cs:86, UndoCommand.cs:28 and VerifyCommand.cs:47. The file asserted the opposite ("a per-file failure does not currently change the exit code"), and its list carried only 0 and 1.

Rather than restate the table, the section now points at Exit Codes, which already carries the full contract, and keeps only what is operational: 0 and 2 both mean the command ran to completion, so a pipeline reading any non-zero code as "nothing happened" is wrong.

2. Docker at runtime

OPERATIONS.md:78 - This statement is no longer correct now that the new verify command runs Immich decoder inside a Docker image (Docker is required at runtime for verify).

Correct. VerifyTask.cs:354 throws Verification requires the docker command and the Immich image, and README.md states Docker is required for verify. The file said the application "needs no Docker daemon at runtime" and called Docker "a packaging and tooling concern only".

Docker now appears in the tool list as a runtime dependency of verify alone, noting that the preflight exits 1 rather than condemning files when the daemon is unreachable. The packaging-and-tooling framing is kept for every other command, where it is still accurate.

Scope

OPERATIONS.md is the only stale surface. A sweep of every markdown file for exit-code and Docker-runtime prose found README.md already correct on both points, and HISTORY.md already recording the exit-code change as breaking under Version 1.1.

Verification

  • CSharpier Format, .NET Build, dotnet format style --verify-no-changes: clean, 0 of 57 files, 0 warnings.
  • dotnet husky run: pass. dotnet test: 372 passed, 0 failed, 0 skipped.
  • markdownlint-cli2 over **/*.md: 0 issues in 11 files. editorconfig-checker clean. Hub prose_lint.py --diff origin/develop: clean.
  • The #exit-codes anchor resolves: README.md carries ### Exit Codes and its own table of contents links the same anchor.

Two statements were written true and were left behind by the verify command.

The exit-code section said a per-file failure does not change the exit code and
listed only 0 and 1. ExitCode.Failed is 2 and every command returns it, so the
note asserted the opposite of the behavior and the list was missing a code. It
now points at the README table rather than restating it, and keeps only the
operational reading: 0 and 2 both mean the command ran to completion.

The tool section said the application needs no Docker daemon at runtime. verify
runs the Immich decoder inside the immich-server image and its preflight throws
when docker is missing, so Docker is a runtime dependency of that one command.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 3, 2026 18:47
@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.37%. Comparing base (5e2eee0) to head (5bccce6).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop      #46   +/-   ##
========================================
  Coverage    43.37%   43.37%           
========================================
  Files           28       28           
  Lines         3896     3896           
  Branches       306      306           
========================================
  Hits          1690     1690           
  Misses        2154     2154           
  Partials        52       52           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Updates OPERATIONS.md so its operational guidance matches the current runtime behavior of PhotoCleaner after the shared ExitCode contract and the verify command’s Docker-based preflight.

Changes:

  • Replaces the stale exit-code explanation with a pointer to the canonical exit-code contract in README.md#exit-codes, and clarifies the operational meaning of 0 vs 2 vs 1.
  • Corrects the Docker runtime claim by documenting Docker as a runtime dependency for verify only, while keeping the “packaging/tooling only” framing for other commands.

@ptr727
ptr727 merged commit 0335970 into develop Aug 3, 2026
14 checks passed
@ptr727
ptr727 deleted the operations-exit-codes-docker branch August 3, 2026 19:04
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.

2 participants