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

Drop Node.js Maintenance LTS from CI #10511

Merged
merged 2 commits into from
May 18, 2021
Merged

Conversation

ddbeck
Copy link
Collaborator

@ddbeck ddbeck commented May 17, 2021

Since the merge of #7398, the published package has become a thin wrapper around a JSON file, supported by very old versions of Node.js. For development, I don't think we need to continue to test multiple versions of Node.js. Instead, we can commit to supporting a single baseline version of Node and test against that. This PR changes the CI workflow to require only the Active LTS version (presently, Node.js 14).

This has a few of benefits:

  • There's a single, obvious workflow to check for failures during review.
  • It's often the case that there are two Node.js releases in Maintenance LTS status, but there's only one Active LTS. By using only the Active LTS, we'll avoid ambiguity about which version we're testing against.
  • We can adopt more modern JS and Node features internally to BCD (as long as we continue to publish our simple JSON package).
  • CI checks against PRs will be marginally faster. Node 14 CI seems often appears to finish 2-5 seconds faster.

Incidentally, this PR also sets actions/setup-node to stable instead of beta.

This PR is blocked by dropping Maintenance LTS from our branch protection rules. The PR is ready for review, but I don't want to relax the rules until this is approved to merge.

@ddbeck ddbeck added the not ready This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action. label May 17, 2021
@github-actions github-actions bot added the infra Infrastructure issues (npm, GitHub Actions, releases) of this project label May 17, 2021
@ddbeck ddbeck requested a review from Elchi3 May 17, 2021 16:47
@ddbeck ddbeck removed the not ready This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action. label May 18, 2021
@ddbeck
Copy link
Collaborator Author

ddbeck commented May 18, 2021

Thanks for the review, @Elchi3. I've removed the branch protection rule for Maintenance LTS and I'll merge this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra Infrastructure issues (npm, GitHub Actions, releases) of this project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants