Skip to content

refactor(cli): remove and correct stale comments - #185

Merged
PunGrumpy merged 2 commits into
mainfrom
refactor/cli-comments
Oct 23, 2026
Merged

PunGrumpy merged 2 commits into
mainfrom
refactor/cli-comments

Conversation

@PunGrumpy

Copy link
Copy Markdown
Owner

Description

The same comment audit for the CLI package. This PR removes 19 comments that narrate the next line and corrects two that state something false. No code line changes.

Two commits:

  • Removed: "Load config first", "Group by file", "Default scan command", "Re-export core programmatic API" and similar. Two of them were also wrong: "Calculate scores per category once" sits above a count of issues, and "Collect all diagnostics" sits above the map of file contents.
  • Corrected:
    • sanitize.ts said a filename can hold newlines and ANSI escapes "on every platform we run on". Windows forbids control characters in file names, so the comment now names Linux and macOS.
    • skill-install.ts said callers treat a null result as "skill not installed", never as a failure. docker-doctor install prints "Failed to install the skill." and exits 1, so that clause goes.

Kept: comments about behavior outside our code (Node refusing to spawn .cmd shims on Windows, .git being a file in worktrees, a JSON string being a valid YAML scalar, process.exitCode for piped output) and the consent note on AGENT_AUTO_FLAGS. The comments inside the prompts, the spinner and the score animation are left to the companion refactor that names the terminal escape sequences.

Related Issues

N/A. Found by a comment audit of the repository.

Checklist

  • I've reviewed my code
  • I've written tests
  • I've generated a changeset file, if this changes a published package
  • I've updated the docs, if necessary

Screenshots (if applicable)

N/A

Additional Notes

  • Comments only, applied from a manifest of exact comment blocks and checked to change nothing else.
  • Counts unchanged: core 465, CLI 101, scripts 43. ultracite check, typecheck and the generated docs are clean.
  • skip-changeset: nothing user-visible changes.

@PunGrumpy PunGrumpy added pkg: cli Command-line interface in packages/docker-doctor skip-changeset This PR needs no changeset (no user-visible change to @docker-doctor/cli) labels Oct 23, 2026
@vercel

vercel Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docker-doctor Skipped Skipped Oct 23, 2026 2:23am UTC

@changeset-bot

changeset-bot Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 8db14c5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 38 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 33cd7d0b-4d72-48a1-8718-49953c780b70
📥 Commits

Reviewing files that changed from the base of the PR and between fdacfde and 8db14c5.

📒 Files selected for processing: 7
  • packages/docker-doctor/src/agents/sanitize.ts
  • packages/docker-doctor/src/agents/skill-install.ts
  • packages/docker-doctor/src/cli.ts
  • packages/docker-doctor/src/formatters/terminal.ts
  • packages/docker-doctor/src/index.ts
  • packages/docker-doctor/src/text.ts
  • packages/docker-doctor/src/workflow-scaffold.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

The latest Docker Doctor scan for this pull request. Learn more about Docker Doctor.

File Status Issues Updated
Dockerfile Error 1 error, 2 info Oct 23, 2026 2:24am UTC

Score: 84 / 100 · Good

3 issues

Dockerfile

  • error Dockerfile:2 Potential secret found in ENV: 'DB_PASSWORD'. no-secrets-in-env
  • info Dockerfile:1 No HEALTHCHECK instruction found. require-healthcheck
  • info Dockerfile:1 No LABEL metadata was found in this Dockerfile. require-labels

Scanned by Docker Doctor for commit 8db14c5.

@pkg-pr-new

pkg-pr-new Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@docker-doctor/cli@185

commit: 8db14c5

@PunGrumpy PunGrumpy removed the skip-changeset This PR needs no changeset (no user-visible change to @docker-doctor/cli) label Oct 23, 2026
Nineteen comments in packages/docker-doctor/src narrated the next line
("Load config first", "Group by file", "Default scan command") or kept
history the code no longer shows. Two were wrong. "Calculate scores per
category once" sits above a count of issues, and "Collect all
diagnostics" sits above the map of file contents.
sanitize.ts said a filename can hold newlines and ANSI escapes on every
platform we run on. Windows forbids control characters in file names,
so the comment now names Linux and macOS.

skill-install.ts said every caller treats a null result as "skill not
installed", never as a failure. The install command reports it as a
failure and exits 1, so that clause goes.
@PunGrumpy
PunGrumpy force-pushed the refactor/cli-comments branch from 1b29b26 to 8db14c5 Compare October 23, 2026 02:23
@PunGrumpy PunGrumpy added the skip-changeset This PR needs no changeset (no user-visible change to @docker-doctor/cli) label Oct 23, 2026
@PunGrumpy PunGrumpy added skip-changeset This PR needs no changeset (no user-visible change to @docker-doctor/cli) and removed skip-changeset This PR needs no changeset (no user-visible change to @docker-doctor/cli) labels Oct 23, 2026
@PunGrumpy
PunGrumpy merged commit 229abf4 into main Oct 23, 2026
21 of 24 checks passed
@PunGrumpy
PunGrumpy deleted the refactor/cli-comments branch October 23, 2026 02:26

This branch was previously deployed

1 inactive deployment
Preview — 8db14c5a Deployed Oct 23, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: cli Command-line interface in packages/docker-doctor skip-changeset This PR needs no changeset (no user-visible change to @docker-doctor/cli)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant