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: Upgrade automigrations to use new safe helpers #20693

Merged
merged 2 commits into from
Jan 20, 2023

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented Jan 19, 2023

  • This makes the code safer as it does not evaluate properties, just looks for string literals that identify that property

Issue: N/A

What I did

Telescoping #20691 to replace getFieldValue calls with getNameFromPath where applicable.

How to test

  1. build the libraries
  2. run the sb automigrate command where new-frameworks, sveltekit-framework, missing-babelrc, mainjsFramework could be applicable (could be multiple runs, no need for all at the same time) - I did test all of them
  3. The automigrations should work fine
  4. The automigrations should not fail if there are weird things like (where it would fail without these changes where applicable):
{
  framework: {
    name: '@storybook/react',
    options: { foo: require('bar') }
}

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"]

@yannbf yannbf added maintenance User-facing maintenance tasks cli labels Jan 19, 2023
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Looks great! Did not test

@shilman shilman changed the title csf-tools: add helpers to get name from node path Csf-tools: Upgrade automigrations to use new safe helpers Jan 20, 2023
@yannbf yannbf force-pushed the feat/csf-tools-name-helpers branch from 9c541c8 to a11bcbe Compare January 20, 2023 09:24
- This makes the code safer as it does not evaluate properties, just looks for string literals that identify that property
@yannbf yannbf force-pushed the feat/improve-automigration-checks branch from 38a31f6 to f380b27 Compare January 20, 2023 09:33
@yannbf yannbf marked this pull request as ready for review January 20, 2023 10:27
Base automatically changed from feat/csf-tools-name-helpers to next January 20, 2023 10:50
@yannbf yannbf changed the title Csf-tools: Upgrade automigrations to use new safe helpers CLI: Upgrade automigrations to use new safe helpers Jan 20, 2023
@yannbf yannbf merged commit d419c08 into next Jan 20, 2023
@yannbf yannbf deleted the feat/improve-automigration-checks branch January 20, 2023 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants