Skip to content

Conversation

@valentinpalkovic
Copy link
Contributor

@valentinpalkovic valentinpalkovic commented Nov 21, 2025

Closes #

What I did

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>

Summary by CodeRabbit

  • Chores
    • Standardized debug log file path messages across the application for improved clarity and consistency.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 21, 2025

📝 Walkthrough

Walkthrough

The pull request standardizes debug log output messages across multiple entry points from "Storybook debug logs can be found at:" to "Debug logs are written to:". Additionally, LogTracker's writeToFile method is modified to always return absolute file paths instead of conditionally returning relative paths in non-CI environments.

Changes

Cohort / File(s) Summary
Log Message Standardization
code/core/src/bin/core.ts, code/lib/cli-storybook/src/bin/run.ts, code/lib/create-storybook/src/commands/FinalizationCommand.ts, code/lib/create-storybook/src/initiate.ts
Updated user-facing log messages from "Storybook debug logs can be found at: ${logFile}" to "Debug logs are written to: ${logFile}". FinalizationCommand also adds a prompt to check the log file for error details.
Logger Implementation
code/core/src/node-logger/logger/log-tracker.ts
Removed isCI import and its usage. Simplified path import to only use named import (join). Changed writeToFile return value to always return absolute logFilePath instead of CI-aware conditional logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

  • Largely repetitive message updates across multiple files with consistent wording changes
  • LogTracker behavior change is straightforward (removal of CI-aware conditional logic) but verify that always returning absolute paths doesn't break dependent code or CI workflows
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a0e40e2 and b3d2ce1.

📒 Files selected for processing (5)
  • code/core/src/bin/core.ts (2 hunks)
  • code/core/src/node-logger/logger/log-tracker.ts (2 hunks)
  • code/lib/cli-storybook/src/bin/run.ts (2 hunks)
  • code/lib/create-storybook/src/commands/FinalizationCommand.ts (1 hunks)
  • code/lib/create-storybook/src/initiate.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.

Applied to files:

  • code/lib/create-storybook/src/initiate.ts
🧬 Code graph analysis (3)
code/lib/create-storybook/src/commands/FinalizationCommand.ts (2)
code/core/src/node-logger/index.ts (2)
  • logger (49-74)
  • logTracker (8-8)
code/core/src/node-logger/logger/log-tracker.ts (1)
  • logTracker (95-95)
code/lib/create-storybook/src/initiate.ts (1)
code/core/src/node-logger/index.ts (1)
  • logger (49-74)
code/lib/cli-storybook/src/bin/run.ts (1)
code/core/src/node-logger/index.ts (1)
  • logger (49-74)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: normal
  • GitHub Check: Core Unit Tests, windows-latest
  • GitHub Check: get-parameters
🔇 Additional comments (9)
code/lib/create-storybook/src/commands/FinalizationCommand.ts (2)

66-68: LGTM: Improved user guidance.

The expanded warning message now explicitly directs users to check the log file for details, which enhances the debugging experience for users encountering non-blocking errors.


72-72: LGTM: Consistent log message standardization.

The updated wording is more concise and aligns with the standardized log messages being applied across the codebase.

code/lib/create-storybook/src/initiate.ts (1)

126-126: LGTM: Standardized log message.

The wording change is consistent with the standardization effort across the codebase.

code/core/src/bin/core.ts (2)

31-31: LGTM: Standardized log message.

The updated wording aligns with the standardization of debug log messages across the application.


70-70: LGTM: Standardized log message.

The updated wording is consistent with the standardized log messages being applied throughout the codebase.

code/lib/cli-storybook/src/bin/run.ts (2)

40-40: LGTM: Standardized log message.

The wording change is consistent with the standardization of debug log messages across the CLI tooling.


83-83: LGTM: Standardized log message.

The updated wording aligns with the standardized debug log messages being applied throughout the codebase.

code/core/src/node-logger/logger/log-tracker.ts (2)

2-2: LGTM: Cleaner import.

Using a named import for join is more specific and follows best practices by importing only what's needed.


91-91: LGTM: Simplified behavior with always-absolute paths.

The change from conditionally returning relative paths (in non-CI) to always returning absolute paths simplifies the logic and provides consistent behavior across environments. This aligns well with the standardized log message "Debug logs are written to:" which is location-neutral.

Benefits:

  • Removes CI-specific branching logic
  • Absolute paths are unambiguous and clearer
  • Consistent behavior in all environments
  • Aligns with the new standardized messaging

Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link

nx-cloud bot commented Nov 21, 2025

View your CI Pipeline Execution ↗ for commit b3d2ce1

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 46s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-21 15:23:38 UTC

Copy link
Member

@yannbf yannbf left a comment

Choose a reason for hiding this comment

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

Thanks <3

@valentinpalkovic valentinpalkovic merged commit edf82eb into next Nov 21, 2025
64 of 71 checks passed
@valentinpalkovic valentinpalkovic deleted the valentin/fine-tune-debug-log branch November 21, 2025 18:28
@github-actions github-actions bot mentioned this pull request Nov 23, 2025
10 tasks
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.

3 participants