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

Adding in Github Actions for CI. #381

Merged
merged 5 commits into from
Jun 11, 2020

Conversation

trankin
Copy link
Contributor

@trankin trankin commented Jun 5, 2020

Linux seems fine, Windows was failing on node_js versions 8.x, 10.x and 11.x. I've removed those in this pull request as I'm not sure if the problem is on the github side or the build code in the repository.

…g on node_js versions 8.x, 10.x and 11.x. I've removed those in this pull request as I'm not sure if the problem is on the github side or the build code in the repository.
.github/workflows/linux.yml Outdated Show resolved Hide resolved
.github/workflows/linux.yml Outdated Show resolved Hide resolved
.github/workflows/linux.yml Show resolved Hide resolved
.github/workflows/windows.yml Outdated Show resolved Hide resolved
@robertsLando
Copy link
Member

windows build is failing because you haven't installed OZW

@trankin trankin requested a review from robertsLando June 8, 2020 14:20
.github/workflows/linux.yml Outdated Show resolved Hide resolved
.github/workflows/linux.yml Outdated Show resolved Hide resolved
.github/workflows/windows.yml Outdated Show resolved Hide resolved
.github/workflows/windows.yml Outdated Show resolved Hide resolved
@trankin
Copy link
Contributor Author

trankin commented Jun 8, 2020

windows build is failing because you haven't installed OZW

I believe part of the windows node-gyp build is to is to build and install OZW correct? It's working on node 12.

@robertsLando
Copy link
Member

I believe part of the windows node-gyp build is to is to build and install OZW correct

You are right (never used it under windows but by checking the code it seems to do it automatically)

So I think something fails in the install process, can you show me the error?

@trankin
Copy link
Contributor Author

trankin commented Jun 8, 2020

I believe part of the windows node-gyp build is to is to build and install OZW correct

You are right (never used it under windows but by checking the code it seems to do it automatically)

So I think something fails in the install process, can you show me the error?

Are you able to see this? https://github.com/trankin/node-openzwave-shared/runs/750522182?check_suite_focus=true

@robertsLando
Copy link
Member

@trankin

Error: spawn C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\MSBuild\15.0\Bin\MSBuild.exe ENOENT

@trankin
Copy link
Contributor Author

trankin commented Jun 8, 2020

Likely related to this: actions/runner-images#68 We may need VS2017 for older versions of nodejs. I won't be able to dig in and review this further for a few days, and can't guarantee that I'll come up with a solve in short order (not to say that it's a hard problem to solve, I have some digging to do as this I'm still getting familiar with github actions), you might consider merging this pull request and leave appveyor running until it's resolved to get older versions running in github actions, or I can remove the windows side of the build in github actions altogether until it's resolved. Let me know how you'd like to see this move forward.

@robertsLando
Copy link
Member

@trankin Don't worry thanks for your collaboration I will dig into this next days

Updating to use windows-2016 instead of windows-latest to build for node js 10.x and 11.x
@trankin
Copy link
Contributor Author

trankin commented Jun 10, 2020

I've confirmed that the problem is in the github actions runner. windows-latest fails for node_js versions 8.x, 10.x and 11.x, however windows-2016 builds successfully for 8.x, 10.x and 11.x. I've updated the action to use windows-2016 for 8.x, 10.x and 11.x and windows-latest for 12.x.

Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

LGTM

@robertsLando robertsLando merged commit 3a9954a into OpenZWave:master Jun 11, 2020
@trankin trankin deleted the feature/github_actions branch June 11, 2020 13:38
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