-
Notifications
You must be signed in to change notification settings - Fork 2k
benchmark: add Windows compatibility #3628
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
Conversation
1. use local git clone instead of git archive | tar 2. spawn from projectPath to avoid need to handle drive letter 3. normalize benchmark path within sampleCode Motivation: Windows users should not be required to use WSL to run benchmark (or any other scripts). The library should be cross-platform as possible both at runtime and in development. Furthermore: it may be valuable to benchmark in multiple environemnts, including unix-type environments, WSL, and native Windows runtimes.
✅ Deploy Preview for compassionate-pike-271cb3 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Hi @yaacovCR, I'm @github-actions bot happy to help you with this PR 👋 Supported commandsPlease post this commands in separate comments and only one per comment:
|
This comment has been minimized.
This comment has been minimized.
@yaacovCR I don't understand this one, probably because I'm not using Windows for development.
Don't have actual stats but I expect 99% of users use some variant of Linux in production. |
Windows paths have colons in them, which node interprets as a protocol like “node:” or “file:” i don’t see the value in introducing the correct presumably file: prefix to the import that would handle this when we have the option to run from working directory and use a relative path. If there is an advantage to using an absolute path, I could reconsider.
Unless they are running in browser.
I feel strongly. |
@yaacovCR Something went wrong, please check log. |
actually, it's pretty easy to use `url.pathToFileURL` to import files in node from path
Fixed with edited list above |
I cancelled the job after > 1 hour. From the log, this seems to be from the involuntary context switches, related to #3625 I think we need a different solution to this problem. Ideas:
|
FYI When trying on WSL, I receive the following error:
Meaning the script as is (and even after this PR) does not support WSL. |
Motivation: Windows users should not be required to use WSL to run benchmark (or any other scripts). The library should be cross-platform as possible both at runtime and in development.
Furthermore: it may be valuable to benchmark in multiple environemnts, including unix-type environments, WSL, and native Windows runtimes.