Skip to content
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

Input variables in tasks #63910

Merged
merged 5 commits into from
Dec 3, 2018
Merged

Input variables in tasks #63910

merged 5 commits into from
Dec 3, 2018

Conversation

alexr00
Copy link
Member

@alexr00 alexr00 commented Nov 28, 2018

Fixes #4758

@alexr00
Copy link
Member Author

alexr00 commented Nov 28, 2018

@isidorn, could you take a look at the configurationResolver* files? I made some significant changes to them and I want to make sure you're ok with them.

@alexr00 alexr00 self-assigned this Nov 28, 2018
@isidorn
Copy link
Contributor

isidorn commented Nov 28, 2018

@alexr00 I took a look at the changes you made to the configuratoinResolver files and they look good to me overall. Great job! I also see you added some tests, so I would just verify:

  • all the tests are green -> this seems to be good
  • variables still get resolved for all the cases
    • ${workspaceRoot}
    • ${env:PATH}
    • ${command:SOME_COMMAND}
    • ${config:editor.lineHeight}

Apart from that I would add a bit more comments once you find the time. I know it was like that before, but we could improve this.
Added some minor comments inline. I can do a more thourugh review in a couple of days when I have more time.

@weinand might also want to take a look at the changes.

@weinand
Copy link
Contributor

weinand commented Nov 28, 2018

@alexr00 Please make sure that variables are resolved correctly in all builds

@alexr00 alexr00 merged commit f9703e9 into master Dec 3, 2018
@alexr00 alexr00 deleted the alexr00/tasksWithArgsSquashed branch December 3, 2018 08:25
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Task Runner: Provide input parameters
3 participants