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

build: Upgrade to Node 20 #847

Merged
merged 8 commits into from
Sep 6, 2024
Merged

Conversation

BilalQamar95
Copy link
Contributor

@BilalQamar95 BilalQamar95 commented Aug 16, 2024

Description

  • Regenerated package-lock.json with node v20 and NPM 10.
  • Added .nvmrc with node v20
  • Removed static Node.js version matrix from ci workflow switched to dynamically set the Node.js version from .nvmrc.
  • Added lockfile version check workflow

Second step in the Node 20 upgrade process, See the tracking issue for further information.

@BilalQamar95 BilalQamar95 requested a review from a team as a code owner August 16, 2024 10:06
@@ -12,17 +12,18 @@ jobs:
matrix:
python-version: ['3.8', '3.12']
django-version: ['4.2']
node: [18]
Copy link
Member

Choose a reason for hiding this comment

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

In the issue for updating all MFEs there is an example PR that configures the ci.yml file so that the workflow is run with both Node 18 and Node 20. Is this no longer the recommended pattern?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have discussed this in FWG and the one you mentioned is the recommended pattern. I'm going to draft this for now and wait for prerequisite PR to get merged

@BilalQamar95 BilalQamar95 changed the title feat: updated node to v20 build: Upgrade to Node 20 Aug 27, 2024
@BilalQamar95 BilalQamar95 self-assigned this Aug 27, 2024
@BilalQamar95
Copy link
Contributor Author

PR will be updated and ready for review once the prerequisite PR for first step of the node v20 upgrade process is merged.

@BilalQamar95 BilalQamar95 marked this pull request as draft August 27, 2024 12:39
@BilalQamar95 BilalQamar95 mentioned this pull request Aug 27, 2024
@BilalQamar95 BilalQamar95 marked this pull request as ready for review August 30, 2024 08:52
@jsnwesson
Copy link
Contributor

Hey @BilalQamar95 , this week I'll be reviewing this PR. Would you mind resolving the package-lock.json conflict?

@BilalQamar95
Copy link
Contributor Author

Hey @BilalQamar95 , this week I'll be reviewing this PR. Would you mind resolving the package-lock.json conflict?

@jsnwesson I have resolved the conflicts

@jsnwesson jsnwesson enabled auto-merge (squash) September 6, 2024 15:27
@jsnwesson jsnwesson merged commit ba51fbd into master Sep 6, 2024
7 checks passed
@jsnwesson jsnwesson deleted the bilalqamar95/node-v20-upgrade branch September 6, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants