-
Notifications
You must be signed in to change notification settings - Fork 54
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, few thoughts:
- Did you you use [
yarn import
(https://classic.yarnpkg.com/en/docs/cli/import/) for the top level package lock? We needed to do this for Simorgh on the call the other day, it may well be necessary for psammead too? I guess the others were regenerated by the existing tooling. - It seems you've wisely chosen to not remove lerna in this PR, I guess we may have a follow-up PR to either
- Remove lerna and replicate it's functionality using yarn's built in features
- Or investigate our existing issues with lerna and try and get local package linking working for example. A new version of lerna has appeared which gives us a bit more confidence in sticking with lerna: Lerna is largely unmaintained lerna/lerna#2703 (comment)
const ensureYarn = () => { | ||
// If npm is being run, print a message and cause 'npm install' to fail. | ||
if (!process.env.npm_execpath.includes('yarn')) { | ||
console.log( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a lint ignore here to suppress this warning?
tput setaf 1 | ||
echo "\nOh no! You're not using our version of npm, please move to version $desired_npm_version in order to reduce bundling inconsistency" | ||
echo "\nOh no! You're not using our version of yarn, please move to version $desired_yarn_version in order to reduce bundling inconsistency" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you do this on the Simorgh branch, I don't remember seeing it, the same script is in that repo too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep this was in the original Simorgh PR
@andrewscfc yeh I used Yeh this PR is just to use Yarn for installing dependencies and switching to Yarn commands. Anything else will be separate PRs. |
@@ -22,10 +22,10 @@ jobs: | |||
scope: '@bbc' | |||
|
|||
- name: Install Node Modules | |||
run: npm run ci:packages | |||
run: yarn run ci:packages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's small and doesn't make much different but I think with yarn you can also remove the run
command so just yarn ci:packages
should work.
Not crucial to this work though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed run
from all yarn commands in this commit
@@ -17,7 +17,7 @@ jobs: | |||
persist-credentials: false | |||
|
|||
- name: Install Node Modules | |||
run: npm run ci:packages | |||
run: yarn run ci:packages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the SetupNode GH action comes preinstalled with yarn! Nice 👍
Bump package versions for psammead, psammead-amp-geo, psammead-brand, psammead-bulleted-list, psammead-bulletin, psammead-byline, psammead-caption, psammead-consent-banner, psammead-content-anchor, psammead-copyright, psammead-embed-error, psammead-episode-list, psammead-figure, psammead-grid, psammead-heading-index, psammead-headings, psammead-image-placeholder, psammead-image, psammead-inline-link, psammead-live-label, psammead-media-indicator, psammead-media-player, psammead-most-read, psammead-navigation, psammead-oembed, psammead-paragraph, psammead-play-button, psammead-podcast-promo, psammead-radio-schedule, psammead-script-link, psammead-section-label, psammead-sitewide-links, psammead-social-embed, psammead-story-promo-list, psammead-story-promo, psammead-timestamp, psammead-useful-links, psammead-visually-hidden-text, psammead-timestamp-container, gel-foundations, moment-timezone-include, psammead-assets, psammead-calendars, psammead-detokeniser, psammead-locales, psammead-rich-text-transforms, psammead-storybook-helpers, psammead-styles, psammead-test-helpers, web-vitals
Updates changelogs for psammead, psammead-amp-geo, psammead-brand, psammead-bulleted-list, psammead-bulletin, psammead-byline, psammead-caption, psammead-consent-banner, psammead-content-anchor, psammead-copyright, psammead-embed-error, psammead-episode-list, psammead-figure, psammead-grid, psammead-heading-index, psammead-headings, psammead-image-placeholder, psammead-image, psammead-inline-link, psammead-live-label, psammead-media-indicator, psammead-media-player, psammead-most-read, psammead-navigation, psammead-oembed, psammead-paragraph, psammead-play-button, psammead-podcast-promo, psammead-radio-schedule, psammead-script-link, psammead-section-label, psammead-sitewide-links, psammead-social-embed, psammead-story-promo-list, psammead-story-promo, psammead-timestamp, psammead-useful-links, psammead-visually-hidden-text, psammead-timestamp-container, gel-foundations, moment-timezone-include, psammead-assets, psammead-calendars, psammead-detokeniser, psammead-locales, psammead-rich-text-transforms, psammead-storybook-helpers, psammead-styles, psammead-test-helpers, web-vitals
041ef8f
to
1764d24
Compare
Checkout your storybook preview here http://psammead-preview.tools.bbc.co.uk/4335 |
Resolves #4331
Overall change:
Replaces npm with yarn for package management.
Code changes:
NPM to Yarn command translations:
npm install
->yarn install
install packagesnpm ci
->yarn install --frozen-lockfile
for deterministic builds based on lockfilenpm install --quiet
->yarn install --silent
installs without console noisenpm update
->yarn upgrade
upgrades all packagesnpm install some-package
->yarn add some-package
installs some-packagenpm install --no-package-lock
->yarn install --no-lockfile
do no generate a lock file on installing