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

Editing Toolkit: specify node version in github workflow #45766

Merged
merged 1 commit into from
Sep 20, 2020

Conversation

deBhal
Copy link
Contributor

@deBhal deBhal commented Sep 20, 2020

Changes proposed in this Pull Request

  • This PR updates the version of node used in the editing toolkit workflow

Currently, the Editing toolkit CI is failing:

2020-09-20T22:19:23.6584403Z error wp-calypso@0.17.0: The engine "node" is incompatible with this module. Expected version "^v12.18.4". Got "12.18.3"

This is because #45721 updated the required node version, but we're still using the default 12.18.3 provided by ubuntu 18.04, so we need to pull in a newer version of node in line with our other workflows. (Aside: there is a ubutu 20-04, but it's got the same node, so totally don't need to do that)

Testing instructions

As a circle-CI update, we're really just looking for the CI tests to pass.

I rebased #45747 on top of this change as a test, and it's gone from failing to passing, which looks good.

A thorough test would look similar:

  • push some change to the Editing Toolkit and verify the CI is failing
  • rebase onto this branch and force-push
  • verify CI success.

OTOH, CI is broken, so breaking it again isn't much of a risk...

@deBhal deBhal requested a review from a team as a code owner September 20, 2020 23:03
@matticbot
Copy link
Contributor

@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 20, 2020
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this @deBhal!!

Testing with #45767

Before

Screen Shot 2020-09-21 at 9 40 33 am

After

Screen Shot 2020-09-21 at 9 48 58 am

@deBhal deBhal merged commit 58b0521 into master Sep 20, 2020
@deBhal deBhal deleted the updates/editing-toolkit-node-version branch September 20, 2020 23:52
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 20, 2020
@simison
Copy link
Member

simison commented Sep 21, 2020

cc @Automattic/team-calypso this was missed perhaps when Node.js was updated recently?

@tyxla
Copy link
Member

tyxla commented Sep 21, 2020

I don't think it was intentional. Thanks for taking care of it @deBhal! 🙇

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.

5 participants