Skip to content

Cleanup some unnecessary logs during dev#7142

Merged
craigmichaelmartin merged 1 commit intomainfrom
04-01-fix_route_include-assets_debug_logs_through_verbose_output
Apr 1, 2026
Merged

Cleanup some unnecessary logs during dev#7142
craigmichaelmartin merged 1 commit intomainfrom
04-01-fix_route_include-assets_debug_logs_through_verbose_output

Conversation

@isaacroldan
Copy link
Copy Markdown
Contributor

@isaacroldan isaacroldan commented Apr 1, 2026

WHY are these changes introduced?

The build process was outputting verbose logging messages directly to stdout, which clutters the console output and makes it harder for users to see important information during builds.

WHAT is this pull request doing?

  • Remove the "Executing step" log message that was printed for every build step
  • Replace direct stdout writes with outputDebug calls for config key processing and manifest generation messages
  • Convert informational messages about skipped config keys and generated manifest files to debug-level logging

How to test your changes?

  1. Run a build command that processes config keys and generates manifest files
  2. Verify that the console output is cleaner without the verbose step execution messages
  3. Enable debug logging to confirm the debug messages are still accessible when needed

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@isaacroldan isaacroldan force-pushed the 04-01-fix_route_include-assets_debug_logs_through_verbose_output branch from 996b055 to 206be79 Compare April 1, 2026 10:46
@isaacroldan isaacroldan changed the title fix: route include-assets debug logs through verbose output Cleanup some unnecessary logs during dev Apr 1, 2026
@isaacroldan isaacroldan force-pushed the 04-01-fix_route_include-assets_debug_logs_through_verbose_output branch 2 times, most recently from 924bdd5 to a8ac7f4 Compare April 1, 2026 11:03
@isaacroldan isaacroldan marked this pull request as ready for review April 1, 2026 11:16
@isaacroldan isaacroldan requested a review from a team as a code owner April 1, 2026 11:16
Copilot AI review requested due to automatic review settings April 1, 2026 11:16
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 1, 2026

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

Copy link
Copy Markdown

Copilot AI left a comment

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 reduces build-time console noise in the app build pipeline by removing per-step “Executing step” logging and moving some informational stdout messages (skipped config keys, manifest generation) to debug-level logging.

Changes:

  • Removed Executing step: ... stdout logging from executeStep.
  • Replaced direct stdout.write(...) messages for “no config key value” and “manifest generated” with outputDebug(...).
  • Updated tests to stop asserting on the removed/changed stdout output.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/app/src/cli/services/build/steps/include-assets/generate-manifest.ts Switch “Generated manifest…” message from stdout to debug logging.
packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.ts Switch “No value for configKey…” message from stdout to debug logging.
packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.test.ts Adjust tests to no longer assert on the removed stdout message.
packages/app/src/cli/services/build/steps/include-assets-step.test.ts Remove assertion tied to the config-key stdout message.
packages/app/src/cli/services/build/client-steps.ts Remove per-step “Executing step” stdout logging.
packages/app/src/cli/services/build/client-steps.test.ts Remove test that asserted the per-step stdout logging.

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

Co-authored-by: Claude Code <claude-code@anthropic.com>
@isaacroldan isaacroldan force-pushed the 04-01-fix_route_include-assets_debug_logs_through_verbose_output branch from a8ac7f4 to bc25d3a Compare April 1, 2026 15:46
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.

📐 Design: The function takes both context: BuildContext (inside config) and a separate options: {stdout} parameter. Since context.options already carries stdout, the second parameter is redundant — every caller passes context.options as the second argument. The inconsistency in the new outputDebug call (reaching for context.options.stdout instead of options.stdout) highlights this duplication.

Suggestion: Consider removing the options parameter and using context.options.stdout (or a local alias) throughout the function. This would require updating the call site in include-assets-step.ts and the test helper, so it may be better suited for a follow-up.

@craigmichaelmartin craigmichaelmartin added this pull request to the merge queue Apr 1, 2026
Merged via the queue into main with commit a0b9284 Apr 1, 2026
41 of 47 checks passed
@craigmichaelmartin craigmichaelmartin deleted the 04-01-fix_route_include-assets_debug_logs_through_verbose_output branch April 1, 2026 17:47
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.

3 participants