-
-
Notifications
You must be signed in to change notification settings - Fork 19
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: Add CI #10
feat: Add CI #10
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lovely. The Windows build is not working BTW because it doesn't have bash installed by default (it runs powershell): https://github.com/exercism/vlang/actions/runs/3446525018/jobs/5751533877
I'd either remove Windows, or add a PowerShell or batch script file specific to Windows.
Let me know which you prefer.
Ah yep. Weird that it fails quietly like that. I'll just pull windows off, as long as it runs in a bash/zsh environment it should be okay |
Bash is installed in the Windows GitHub Actions environment. It's just that PowerShell is the default. Better approach: if the script works on Windows, we should be able to run it on all 3 platforms by specifying bash explicitly. |
Ah nice good to know, thank you! |
Apply suggestions from code review Co-authored-by: ee7 <45465154+ee7@users.noreply.github.com>
No problem. Looks like the Windows build passes now - nice. Just in case it's handy, the source of truth for what's installed on the Windows image is here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. But I haven't tested bin/test
locally.
Some small nitpicks.
security and shellcheck considerations Co-authored-by: ee7 <45465154+ee7@users.noreply.github.com>
Great! |
It is working on all architectures on my fork