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

ci: pin node to under 20.5.1 #1575

Closed
wants to merge 2 commits into from

Conversation

buffalojoec
Copy link
Collaborator

I spotted this in one of the PRs linked in nodejs/node#49497 .

It should solve our CI problem until the issue is resolved (?).

@mergify mergify bot added the community label Sep 7, 2023
@mergify mergify bot requested a review from a team September 7, 2023 21:02
@buffalojoec buffalojoec marked this pull request as ready for review September 7, 2023 21:20
@buffalojoec buffalojoec marked this pull request as draft September 7, 2023 21:39
@steveluscher steveluscher added the automerge Merge this Pull Request automatically once CI passes label Sep 7, 2023
@steveluscher
Copy link
Collaborator

I'm going to presume we can preemptively allowlist >=20.6.1. Let's see if CI is OK with that.

@steveluscher steveluscher marked this pull request as ready for review September 7, 2023 21:41
@mergify mergify bot removed the automerge Merge this Pull Request automatically once CI passes label Sep 7, 2023
@mergify
Copy link
Contributor

mergify bot commented Sep 7, 2023

Automerge label removed due to a CI failure

@buffalojoec
Copy link
Collaborator Author

I'm going to presume we can preemptively allowlist >=20.6.1. Let's see if CI is OK with that.

Didn't work on mine either, weird.

@steveluscher
Copy link
Collaborator

Didn't work on mine either, weird.

Well, it depends on what you mean by ‘work.’ The engines thing doesn't affect this, I don't think:

matrix:
node:
- 'current'
- 'lts/*'

That's the only thing that will fix CI. The engines thing is for people who install us, I think.

@steveluscher
Copy link
Collaborator

I wouldn't worry about it too much. Let's just let them ship nodejs/node#49528.

@2501babe
Copy link
Member

2501babe commented Sep 7, 2023

perhaps temporarily remove the rule where a failure on node current aborts the run on node lts? assuming this is trivial anyway

@buffalojoec
Copy link
Collaborator Author

That's the only thing that will fix CI. The engines thing is for people who install us, I think.

This does sound more logical ha

@buffalojoec buffalojoec closed this Sep 9, 2023
@github-actions
Copy link
Contributor

Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants