-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix Windows build and build on Windows CI #74
Conversation
Could a way of checking the environment variables independently of the operating system be by introducing either a flag or command to return the environment variables that the CLI has access to? This means you don't have to worry about if Microsoft changes their commands, and guarantees that the environment variables returned are the same ones that the command actually has access to. A flag would allow it to be used with any of the commands so that you can still test the |
04eaa08
to
44227e0
Compare
Yes, I was thinking about that. For now I'm going to disable these tests on Windows and work on enabling them in a next PR |
785a75a
to
ad0f067
Compare
ad0f067
to
a654bcc
Compare
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.
👏
@chriso please let me know if I should do something to kill all the processes on Windows similar to how it's done for linux and darwin. @achille-roussel not sure how to use
...so I reuse the same code in |
The reason we use the lower level The higher level I'm not familiar with Windows syscalls so I'm not sure how to accomplish the same thing there. Windows may not even have the concept of process groups. I think what you've done here – default to using |
We should use something platform-independent instead of
printenv
. I'm going to work on this in another PR