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
feat: add a new .taskrc.yml to enable experiments #1982
base: main
Are you sure you want to change the base?
feat: add a new .taskrc.yml to enable experiments #1982
Changes from 8 commits
cddf64e
fcc91c7
6b6da0c
4175ef8
18b1511
c88884f
ca28e5d
d87b7d5
dda6a5a
5bf8fa9
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.
Question: Currently the only acceptable values for experiments are integers (1, 2). However, technically the value is saved as a string (mostly because env vars are always strings). If we think that we will never use anything other than integers, then maybe the values in the config file should be ints:
Open to ideas about why we'd want to support strings though.
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.
@pd93 Good question!
Since experiment values are typed as strings (because environment variables are strings), I considered two options:
Experiments map[string]string
withyaml:"experiments"
asmap[string]string
, allowing the parser to handle the conversion. This approach lets me keep the existing code.Experiments
asmap[string]int
and modify the code to convert all environment variables to integers.I choose the first one to keep the existing code but you may be right, we could / should define that experiment should be only int
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've converted everything to int :)