-
Notifications
You must be signed in to change notification settings - Fork 151
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
Use the appropriate quote in Windows (#393) #473
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.
CI reports a long line. Also, could you add a comment linking to the issue where this is discussed? The code is very confusing to anyone who knows how process invocation is supposed to work, unless they have that context.
Will do. I’ll probably get to that in Sunday |
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.
Thanks!
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.
Oh, actually CI is still failing.
I pushed another fix. Seems to be okay now |
Sorry for bugging, but is there anything else that I should do before we can merge? |
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.
Nope, it's all good. Thank you for your patience.
This is probably not the ideal solution, but it makes stuff work until someone come up with a better one