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

Run tests & prebuild for node 16 #21

Merged
merged 1 commit into from
Jun 7, 2021
Merged

Conversation

pimterry
Copy link
Contributor

@pimterry pimterry commented Jun 1, 2021

This PR changes the specific node versions that are tested and prebuilt, to add support for node 16.

This also stops pinning v14 builds to v14.2 (originally pinned in #16 (comment)). That was due to an appveyor issue that seems to now be resolved.

In the medium term I expect that moving to N-API would be good, if that's possible, so none of this is necessary, but in the short term updates like this are quick and easy 😃.

@pimterry
Copy link
Contributor Author

pimterry commented Jun 1, 2021

Test errors here are a failure to resolve nodejs.org and a network timeout, I think appveyor is just having a blip.

I tested it separately on appveyor a couple of times before this PR, and it does all build successfully: https://ci.appveyor.com/project/pimterry/win-version-info/builds/39409627.

@vweevers
Copy link
Owner

vweevers commented Jun 2, 2021

In the medium term I expect that moving to N-API would be good

That's what I prefer as well, it's only blocked by lack of time atm.

but in the short term updates like this are quick and easy

👍

I think appveyor is just having a blip

Probably yes. I triggered a retry.

@pimterry
Copy link
Contributor Author

pimterry commented Jun 4, 2021

I triggered a retry.

All passing now! @vweevers Looks like this is all good to merge & release if you're happy with it 👍

@vweevers vweevers merged commit 764c868 into vweevers:master Jun 7, 2021
@vweevers
Copy link
Owner

vweevers commented Jun 7, 2021

Release build failed - https://ci.appveyor.com/project/vweevers/win-version-info/builds/39500373/job/e4b3kvcbknxi233t - unrelated to this PR because I also upgraded prebuild-install (4.0.0). At first glance it looks like it's attempting (and failing) to build a beta release of electron for some reason. Will look at that later.

@vweevers
Copy link
Owner

vweevers commented Jun 7, 2021

(it might be easier to just switch to N-API though 😄)

@pimterry pimterry deleted the update-node branch June 8, 2021 10:40
@pimterry
Copy link
Contributor Author

pimterry commented Jun 8, 2021

Haha, fun fun fun. Up to you, if switching to N-API is easier than fixing the Electron build then that makes a lot of sense to me!

In case you're looking for even more build admin work 😃 I'd maybe consider switching to GitHub actions if you're swapping everything around anyway? I've been steadily migrating all my appveyor projects onto their windows runners, it's been easy to do and the UX/stability/performance is much better.

@vweevers
Copy link
Owner

vweevers commented Jun 9, 2021

Yes, no question! I'm a fan of GHA too.

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

Successfully merging this pull request may close these issues.

2 participants