Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Text Analytics ARM template for Live tests #7638
Text Analytics ARM template for Live tests #7638
Changes from all commits
b772204
9664b78
649113a
431f1b3
984c04a
2c8beb0
da12c53
9902ed3
6f08995
54cf482
a5d8ced
cd3f336
d8312d0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Let's continue to use the
tests.yml
to handle loading these, no need to output these here.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.
I think the more recommended approach is to only maintain test variables in the tests-resources file as that can serve as a central place for all that info.
@danieljurek Can add more context.
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.
In the case of the variables being passed through the ARM template, this gives us one location to specify environment variables for a live test (compared with mixing environment variables defined in the ARM template and those defined in the
tests.yml
file).Since the ARM template is meant to be used with
New-TestResources.ps1
it is more convenient for contributors to specify the environment variables in one place. The script outputs the set of environment variables needed to run tests and doesn't require the contributor to know to consult thetests.yml
file.Either approach works but over time I've convinced myself that it makes sense to collect the environment variables in one place for the benefit of contributor experience.
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.
As of now, I have used
test-resources.json
to declare variables. Few other languages are also doing it. I think this is one think does not need to differ from other languages and have one common place for variables. So I will vote for keeping in this file.