Skip to content
This repository has been archived by the owner on Dec 5, 2022. It is now read-only.

Fix build.yml so we actually run on Windows and macOS #590

Closed
wants to merge 1 commit into from

Conversation

joliss
Copy link
Contributor

@joliss joliss commented Aug 19, 2020

Right now we just run on ubuntu-latest three times.

See for example https://github.com/covidatlas/li/commit/d06f2ea5fb0a6647ef42b818991d57e6cf29bcb9/checks#step:1:3, which has os = windows-latest, but is actually running on Ubuntu.

I'm assuming this PR will uncover some tests that'll fail on Windows or Mac, so perhaps we should hold off on merging it until those are fixed.

Right now we just run on ubuntu-latest three times.

This patch also sets the shell to bash for all `run` steps, which is
needed for Windows.
@joliss
Copy link
Contributor Author

joliss commented Aug 19, 2020

So it looks like there's a whole bunch of things going wrong on Windows; see the test output on Windows. I don't think this is something I want to try and fix, especially since I don't have a Windows development box set up.

Do we have any volunteers to try and fix this? Or do we just give up on the Windows build for now?

@ryanblock
Copy link
Contributor

Wow, huge find, thank you!

@ryanblock
Copy link
Contributor

ryanblock commented Aug 20, 2020

One thing that occurs to me looking at this: npm config set script-shell "C:\\Program Files\\Git\\bin\\bash.exe" – may want to drop that in favor of just using cross-env, wdyt?

Update: on second thought, prob not, idk!

@jzohrab
Copy link
Contributor

jzohrab commented Aug 20, 2020

Let's give up on the Windows build for now. We don't have any devs on windows, prod isn't windows, and if we do get one who has dev experience, then that can be their first concrete task to work on. I'm running on mac with no issues during local test and dev, which is sufficient for me.

Great find, @joliss . I've created a fix-windows-ci branch using your work, and opened #591. I'll check macOS in a separate branch, and will update master.

@jzohrab
Copy link
Contributor

jzohrab commented Aug 20, 2020

Verifying macOS ci works in a separate branch -- run details are at https://github.com/covidatlas/li/runs/1008043577?check_suite_focus=true.

Opening a new PR which will focus on what works. Thanks @joliss ! jz

@jzohrab
Copy link
Contributor

jzohrab commented Aug 20, 2020

Closing in favor of #592.

@jzohrab jzohrab closed this Aug 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants