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

feat: node 16 compatibility #2199

Merged
merged 4 commits into from May 31, 2021
Merged

feat: node 16 compatibility #2199

merged 4 commits into from May 31, 2021

Conversation

vincent-tr
Copy link
Contributor

@vincent-tr vincent-tr commented May 26, 2021

Node v16 flags seems to have changed.

closes #2199

@gr2m
Copy link
Member

gr2m commented May 26, 2021

Thanks Vincent. Could you add a test that will fail with Node 16, and add node 16 to the CI test matrix?

@vincent-tr
Copy link
Contributor Author

I added a test.
It reproduce the current problem, and adding the first commit fixes it on my computer.
Please report to me if this does not match your testing practices.

I also tried to add node 16 on CI test matrix, but I'm not used to github workflows, so don't hesitate to point changes to do about it.

@gr2m gr2m self-assigned this May 28, 2021
@gr2m
Copy link
Member

gr2m commented May 28, 2021

I couldn't update your fork, so I created a new PR at #2201. I pushed the test commit and CI config update first to see it fail, and will then push your fix to see it pass

@gr2m
Copy link
Member

gr2m commented May 28, 2021

still fails for me, see #2201. Feel free to continue to work on your PR, I can close mine, now that we've shown that tests fail on Node 16

@vincent-tr
Copy link
Contributor Author

I updated the failing test (seems related to node v16.2).
It seems to be green now.

@gr2m gr2m changed the title Adapt to Node v16 feat: node 16 compatibility May 31, 2021
@gr2m gr2m merged commit af05502 into nock:main May 31, 2021
@github-actions
Copy link

🎉 This PR is included in version 13.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@vincent-tr
Copy link
Contributor Author

👍

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

Successfully merging this pull request may close these issues.

None yet

2 participants