Skip to content

key:generate: send the warning to stderr - #12

Merged
necessarylion merged 1 commit into
mainfrom
chore/review-nits
Nov 2, 2026
Merged

necessarylion merged 1 commit into
mainfrom
chore/review-nits

Conversation

@mydoxie

@mydoxie mydoxie commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Problem: the unknown-option warning of key:generate went to stdout. So KEY=$(dox key:generate --show foo) held the warning and the key.

Fix: the warning goes to stderr. A test checks that stdout has only the key with --show. The other output in this command is not a warning: --show returns before the production check, and that check is shared with migration:fresh, so I left it.

Also: a registry test that key:generate foo passes validateArgs. Changelog line updated.

@mydoxie

mydoxie commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Bot review

I ran dart analyze --fatal-infos, dart format check, test/generate_key_test.dart and test/command_registry_test.dart in a separate worktree with sibling clones. All clean, all pass.

Blocking

  • None. With --show, stdout is still only the key: the warning goes to stderr, print(secret) runs and returns before confirmDropAll, and the new process test checks stdout with ^[A-Za-z0-9]{32}\n$. Exit code is unchanged. No other command is touched.

Non-blocking

  • A script that read the warning from stdout will now find it on stderr. That is the point of the fix and the changelog says so.
  • The Stopped. APP_KEY was not changed. message is still on stdout, but it only happens without --show. Fine.
  • The two new process tests start dart run, so each takes a few seconds in CI. Fine.
  • No version: bump. Changelog line is under Unreleased and plain.

@necessarylion
necessarylion merged commit 1d24739 into main Nov 2, 2026
1 check passed
@necessarylion
necessarylion deleted the chore/review-nits branch November 2, 2026 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants