-
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
Remove use of pipefail
, switch to use CustomExecution
, add decoration for unset settings
#85
Conversation
Signed-off-by: Sanjula Ganepola <[email protected]>
Signed-off-by: Sanjula Ganepola <[email protected]>
…tput retrieval Signed-off-by: Sanjula Ganepola <[email protected]>
…s selected but not set Signed-off-by: Sanjula Ganepola <[email protected]>
Signed-off-by: Sanjula Ganepola <[email protected]>
src/act.ts
Outdated
command, | ||
{ | ||
cwd: commandArgs.path, | ||
shell: true, |
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.
Note: This was added due to nodejs/node#7367 (comment)
Hmm... This error is new for me, the change with only removed images worked somehow Got a try on macOS today, seems there is a new bug included here.
branching via |
I wonder if this stuff can be tested on CI? the Myself didn't go so far to test my vscode extensions, but something very useful. |
src/act.ts
Outdated
{ | ||
cwd: commandArgs.path, | ||
shell: true, | ||
env: settings.secrets |
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.
maybe env: [...process.env, ...settings.secrets(the rest)]
So HOME is never undefined
Signed-off-by: Sanjula Ganepola <[email protected]>
Signed-off-by: Sanjula Ganepola <[email protected]>
Signed-off-by: Sanjula Ganepola <[email protected]>
Interesting idea. I have some ideas for vscode testing which I have done previously. We can have a look at setting up a test framework once these bugs are resolved. @ChristopherHX Could you have another try with this PR? I have pushed some more updates and confirmed it worked on Windows and Linux. I am unable to test on MacOS. |
@ChristopherHX PR #70 in fact actually builds on top of this one to address some other issues as well so it might be easier to just test with that branch. |
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.
pr #70 works fine so far
Changes
CustomExecution
for VS Code task to avoid use ofpipefail
but still be able to retrieve execution output