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 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
… to be more readable.
@robertsLando
Copy link
Member

robertsLando commented Jun 8, 2020

windows build is failing because you haven't installed OZW

@trankin trankin requested a review from robertsLando Jun 8, 2020
.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

robertsLando 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?

@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

robertsLando commented Jun 8, 2020

@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/virtual-environments#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

robertsLando commented Jun 9, 2020

@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.

@trankin trankin requested a review from robertsLando Jun 10, 2020
Copy link
Member

robertsLando left a comment

LGTM

@robertsLando robertsLando merged commit 3a9954a into OpenZWave:master Jun 11, 2020
1 check passed
1 check passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@trankin trankin deleted the trankin:feature/github_actions branch Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.