Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI: Remove random commas in storybook upgrade logs #22333

Merged
merged 4 commits into from Aug 29, 2023

Conversation

joaonunomota
Copy link
Contributor

@joaonunomota joaonunomota commented May 1, 2023

Closes #22298

What I did

Log the stdout and stderr properties, instead of logging output property of result returned by npm-check-updates@latest.

This provides the following benefits:

  • Avoids commas that come from logging an array, which solves [Bug]: some extra comma #22298.
  • It is clearer in the code what is being logged, since stdout and stderr are equal to output[1] and output[2] respectively.
  • stderr can be logged at a different level to stdout (not sure what the rule for using info or warn on logs)

It also:

  • Does not log output[0] which in this case is null.
  • Adds an extra newline between stdout and stderr logs

An example of what the logs look like before and after with some forced warnings (using reproduction code):

image

How to test

  1. Run a sandbox for template, e.g. yarn task --task sandbox --start-from auto --template react-vite/default-ts
  2. Run npx storybook@latest upgrade
  3. Observe logs, specifically for npm-check-updates

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

Prevents commas from being logged on the terminal
Adds newline between stdout and stderr
@joaonunomota joaonunomota changed the title Log stdout and stderr from npm-check-updates separately Fix: Remove random commas in storybook upgrade logs May 1, 2023
Copy link
Contributor

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

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

Thanks for this! 💪

@JReinhold JReinhold changed the title Fix: Remove random commas in storybook upgrade logs CLI: Remove random commas in storybook upgrade logs Aug 29, 2023
@JReinhold JReinhold merged commit d23d577 into storybookjs:next Aug 29, 2023
50 of 58 checks passed
@joaonunomota joaonunomota deleted the no-more-commas branch August 29, 2023 11:21
@yannbf yannbf mentioned this pull request Oct 17, 2023
5 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.

[Bug]: some extra comma
2 participants