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

ci: change action used for storybook deploy #2843

Merged
merged 4 commits into from
Mar 22, 2024

Conversation

jpandersen87
Copy link
Contributor

@jpandersen87 jpandersen87 commented Mar 21, 2024

Summary

Changes the github action used for storybook deployment to one suggested by Storybook in their documentation on deployment to Github Pages (see: https://storybook.js.org/docs/sharing/publish-storybook#github-pages) in order to hopefully resolve errors in the current storybook deployment.

@jpandersen87 jpandersen87 requested a review from a team as a code owner March 21, 2024 22:17
@jpandersen87 jpandersen87 changed the title fix: change action used for deploy fix: change action used for storybook deploy Mar 21, 2024
@@ -42,9 +47,9 @@ jobs:
run: yarn build-storybook
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we need this step still? (And the yarn install above it too?)

Looks like this github action does both of those for us

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree on this feedback, not sure we need the steps Brandon noted. Can we trigger this build before merging it? I am not sure and I know that's not always possible with GHA.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yeah, actually, i just duplicated the branch to our repo and it's now available in the Actions tab
Screenshot 2024-03-22 at 11 46 18 AM

Should I run it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yarn install/build-storybook steps have been removed

Copy link
Contributor

Choose a reason for hiding this comment

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

@jpandersen87 it was easier for us to test and iterate on a Truss-owned branch. If you make your PR diff match https://github.com/trussworks/react-uswds/pull/2844/files, I'm happy to approve so you get the credit!

Thanks for taking the time! PS: Our latest storybook deploy is live thanks to this change: https://trussworks.github.io/react-uswds/

Copy link
Contributor

Choose a reason for hiding this comment

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

@jpandersen87 you're so fast. Literally did this as/before I recommended it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

haha, I was watching the deploy and saw the node16 warnings and made the action version update changes literally the same time as you did

@werdnanoslen werdnanoslen changed the title fix: change action used for storybook deploy ci: change action used for storybook deploy Mar 22, 2024
@brandonlenz
Copy link
Contributor

@all-contributors please add @jpandersen87 for infra

Copy link
Contributor

@brandonlenz

I've put up a pull request to add @jpandersen87! 🎉

@brandonlenz brandonlenz merged commit 9bfba17 into trussworks:main Mar 22, 2024
6 checks passed
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.

4 participants