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 CI for PRs on master #688

Merged
merged 3 commits into from
Aug 16, 2023
Merged

Fix CI for PRs on master #688

merged 3 commits into from
Aug 16, 2023

Conversation

micgro42
Copy link
Collaborator

@micgro42 micgro42 commented Aug 15, 2023

This does three things:

  1. it makes sure we explicitly use our lerna, not the global one that seems to come with ubuntu_latest in the build step
  2. it makes sure we use the storybook CLI that matches our storybook version when extracting the stories, npx sb would install the latest version if it is not already in node_modules
  3. it updates chromatic because there was an issue with the chromatic part failing despite a change already being accept in the chromatic web interface (this PR does not touch components at all)

@github-actions
Copy link

If the binary to execute does not exist in `node_modules/.bin/`, then
npx will install the package. In the current setup, that would install
@storybook/cli 7.x
@@ -57,7 +57,7 @@ jobs:
run: npm ci
- name: Build # only runs in case of cache hit, otherwise as part of npm ci
if: steps.cache.outputs.cache-hit == 'true'
run: lerna bootstrap
run: npm run lerna bootstrap
Copy link
Contributor

Choose a reason for hiding this comment

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

i see, good catch!

@micgro42 micgro42 changed the title Explicitly use lerna from our node_modules, not global Fix CI for PRs on master Aug 15, 2023
@micgro42 micgro42 marked this pull request as ready for review August 15, 2023 16:57
@micgro42 micgro42 merged commit c5fb2ab into master Aug 16, 2023
5 checks passed
@micgro42 micgro42 deleted the chore/figureOutLerna branch August 16, 2023 08:25
micgro42 added a commit that referenced this pull request Aug 16, 2023
This is the equivalent of #688 for the `next` branch. However, here we do not need the chromatic update, because we're already on Chromatic v6.
micgro42 added a commit that referenced this pull request Aug 16, 2023
If that is not explicitly set, CI will use whatever comes with `ubuntu_latest` and that is not what we want for this workflow. Node 14 is consistent with what is used in the other workflows. The PR to upgrade these is #684.

This should be merged after #688 and make CI green again on merged branches.
micgro42 added a commit that referenced this pull request Aug 16, 2023
When this action runs, `hashFiles('*/dist')` would always return the empty string, because it does not match any files. That means that the cache key would always be identical: `linux-`.

This should be merged after #688
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants