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

Update Yarn, Node and dependencies #1145

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

taliesins
Copy link

Description

Updated to the Yarn 4.1.1, Node 20 and update package dependencies

Also update Dockerfiles and github workflow steps to use the latest versions.

Needed to update to the latest version of Yarn sytanx

Which issue(s) does this PR fix

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

@taliesins taliesins requested a review from a team as a code owner March 27, 2024 16:15
Copy link

openshift-ci bot commented Mar 27, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign zaperex for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@davidfestal
Copy link
Member

For a number of reasons, I'm not sure we want to move to yarn 4 for now.

@nickboldt @kadel Do you confirm ?

@kadel
Copy link
Member

kadel commented Mar 27, 2024

For a number of reasons, I'm not sure we want to move to yarn 4 for now.

Yarn 4 is definitely on our roadmap, but we can't update it right now due to limitations in our build system. We need to resolve those first.

@kadel
Copy link
Member

kadel commented Mar 27, 2024

There should not be any external blockers for moving to node 20, but there were some fixable issues. @BethGriggs was looking into that.

@taliesins
Copy link
Author

Which limitations in the build system? I think I managed to fix all of the local builds and Docker ones?

Copy link

sonarcloud bot commented Mar 27, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots
3.1% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

Copy link
Contributor

The image is available at: quay.io/janus-idp/backstage-showcase:pr-1145!

@taliesins
Copy link
Author

@davidfestal and @kadel I have fixed the build part now. What do I need to do about getting SonarCloud gate passing as its not really code I have touched.

Copy link
Member

@nickboldt nickboldt left a comment

Choose a reason for hiding this comment

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

Unfortunately we cannot move to Yarn v4 at this time, for two reasons:

  • downstream RHDH build process requires Yarn v1 today and v3 in future
  • upstream Backstage community-plugins repo will use Yarn v3, not 4.

Can you revise this PR to use Node 20, but stick with yarn 1.22.22 ?

cc: @schultzp2020

@davidfestal
Copy link
Member

Can you revise this PR to use Node 20, but stick with yarn 1.22.22 ?

Please be aware that the work of migrating to Node 20 has already been implemented in the following PR: #1048

@davidfestal
Copy link
Member

Can you revise this PR to use Node 20, but stick with yarn 1.22.22 ?

Please be aware that the work of migrating to Node 20 has already been implemented in the following PR: #1048

cc @BethGriggs

@taliesins
Copy link
Author

taliesins commented Mar 28, 2024

The whole thing that started me down this path was the large amount of time that it took to build this solution so that I could containerize it. On my work machines with security tools it was taking me 58 minutes to build. I had to remote into my home machine (4th gen nvme, 128GB of ram & Ryzen 5950x) to build anything. Yarn 4 is significantly faster then Yarn 1 on my machine.

Have a look at another PR like #1133 it took 31 minutes vs the 17 minutes this one takes for the "PR Docker Build / PR Docker Build (pull_request_target)" step.

Steps on GitHub run with hardened mode on as well. For local installs it feels even faster. See https://yarnpkg.com/blog/release/4.0 for section of performance.

@openshift-merge-robot
Copy link

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to Node 20
5 participants