Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Indicate Node.js supported versions #6180

Merged
merged 3 commits into from
Sep 7, 2023

Conversation

legobeat
Copy link
Contributor

@legobeat legobeat commented Sep 3, 2023

PR description

Testing instructions

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if documentation updates are required.

Breaking changes and new features

  • I have added any needed breaking-change and new-feature labels for the appropriate packages.

@haltman-at
Copy link
Contributor

Hm, do we want to set .nvmrc to lts/*? That's what it is now, but we've often lagged behind a bit.

@legobeat
Copy link
Contributor Author

legobeat commented Sep 5, 2023

How about pinning to lts as of now, 18?

I guess it's mostly a matter of if you want future upgrades to be explicit or not.

@haltman-at
Copy link
Contributor

Oh, well right now we're still supporting 16, so...?

@legobeat
Copy link
Contributor Author

legobeat commented Sep 5, 2023

@haltman-at Note that the value in .nvmrc does not affect which version unit tests are being run in (which still includes 16) - only the yarncheck job is being changed to use .nvmrc here.

Copy link
Contributor

@gnidan gnidan left a comment

Choose a reason for hiding this comment

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

This approach is probably what we should've been doing all along.

For that .nvmrc, I think we probably would want that yarncheck job to run on the earliest Node version we support, but it probably doesn't matter too much.

One approve from me :) Thank you!

Copy link
Contributor

@haltman-at haltman-at left a comment

Choose a reason for hiding this comment

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

Well, if Nick says it's good, then I'm also OK with it!

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

Successfully merging this pull request may close these issues.

3 participants