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

Fix storybook build #7955

Merged
merged 1 commit into from
Mar 1, 2022
Merged

Fix storybook build #7955

merged 1 commit into from
Mar 1, 2022

Conversation

roryabraham
Copy link
Contributor

Details

Fixes failed web deploy: https://github.com/Expensify/App/runs/5380104817?check_suite_focus=true

Fixed Issues

$ n/a

Tests (dev)

  1. Run npm run storybook – it should work!
  2. Run npm run storybook-build – it should work!
  • Verify that no errors appear in the JS console

PR Review Checklist

Contributor (PR Author) Checklist

  • I made sure to pull main before submitting my PR for review
  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I clearly indicated the environment tests should be run in (Staging vs Production)
  • I wrote testing steps that cover success & fail scenarios (if applicable)
  • I ran the tests & they passed on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified there are no console errors related to changes in this PR
  • I followed proper code patterns (see Reviewing the code)
    • I added comments when the code was not self explanatory
    • I put all copy / text shown in the product in all src/languages/* files (if applicable)
    • I followed proper naming convention for platform-specific files (if applicable)
    • I followed style guidelines (in Styling.md) for all style edits I made
  • I followed the guidelines as stated in the Review Guidelines

PR Reviewer Checklist

  • I verified the Author pulled main before submitting the PR
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the testing environment is mentioned in the test steps
  • I verified testing steps cover success & fail scenarios (if applicable)
  • I verified tests pass on all platforms & I tested again on all platforms
  • I checked that screenshots or videos are included for tests on all platforms
  • I verified there are no console errors related to changes in this PR
  • I verified proper code patterns were followed (see Reviewing the code)
    • I verified comments were added when the code was not self explanatory
    • I verified any copy / text shown in the product was added in all src/languages/* files (if applicable)
    • I verified proper naming convention for platform-specific files was followed (if applicable)
    • I verified style guidelines were followed
  • I verified that this PR follows the guidelines as stated in the Review Guidelines

QA Steps (staging)

  1. Go to https://staging.new.expensify.com/docs/index.html
  2. The documentation page should load.
  • Verify that no errors appear in the JS console

Tested On

Web (Storybook) only

Screenshots

Web

image

@roryabraham roryabraham requested a review from a team as a code owner March 1, 2022 20:14
@roryabraham roryabraham self-assigned this Mar 1, 2022
@botify
Copy link

botify commented Mar 1, 2022

Hey, I noticed you changed some webpack configuration files. This can break production builds. Did you remember to run a production build locally to verify they still work?

@MelvinBot MelvinBot removed the request for review from a team March 1, 2022 20:14
@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2022

⚠️ ⚠️ Heads up! This pull request has the CP Staging label. ⚠️ ⚠️
Merging it will cause it to be immediately deployed to staging, even if the open StagingDeployCash deploy checklist is locked.

@roryabraham
Copy link
Contributor Author

cc @kidroca

Copy link
Contributor

@luacmartins luacmartins left a comment

Choose a reason for hiding this comment

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

LGTM and tests well. Thanks for the fix @roryabraham!

@yuwenmemon yuwenmemon merged commit ed1d130 into main Mar 1, 2022
@yuwenmemon yuwenmemon deleted the Rory-FixStorybookBuild branch March 1, 2022 21:15
OSBotify pushed a commit that referenced this pull request Mar 1, 2022
Fix storybook build

(cherry picked from commit ed1d130)
@OSBotify
Copy link
Contributor

OSBotify commented Mar 1, 2022

🚀 Cherry-picked to staging by @yuwenmemon in version: 1.1.41-1 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

@kidroca kidroca mentioned this pull request Mar 3, 2022
35 tasks
@OSBotify
Copy link
Contributor

🚀 Deployed to production by @francoisl in version: 1.1.41-6 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

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.

6 participants