Rename "native" runtime/runner to "ambient"#224
Merged
Hidden character warning
The head ref may contain hidden characters: "trs/native\u2192ambient"
Conversation
victorlin
approved these changes
Oct 7, 2022
Member
victorlin
left a comment
victorlin
left a comment
There was a problem hiding this comment.
Manual testing:
-
nextstrain build --nativestill works - having
runner = nativein~/.nextstrain/configstill works
Looks good! Just a few minor things.
tsibley
force-pushed
the
trs/native→ambient
branch
from
c7ee3cc to
b160fc5
Compare
The incompatibility of --image is no longer limited to just the "native" runner.
One piece still suitable for usage with argparse's "type" and another suitable for internal use. For the latter, we don't want to be importing from nextstrain.cli.argparse or catching ArgumentTypeErrors.
…nvalid name is given This sort of hint is super helpful to users.
…instead of handling it in a one-off way. The initial reason for doing something else here was so "choices" could be used, but that reason is moot now that I've included the list of valid names in the error messages produced by runner_module_argument(). It's good for usability to maintain consistency across commands in handling of arguments. This consistency also means `setup` will now accept unnormalized names like "AWS-Batch" and future aliases. Most changes in the diff are simple "runner" → "opts.runner" swaps due to the type change of the latter meaning the former is no longer necessary.
This name better reflects what it is and has been a long time coming. "Native" was never a name I was happy with, even when I introduced it. The more descriptive name will help us explain what it is in documentation and also further distinguish it from the new Conda runtime, which is also "native" in the binary executable sense. Continue to accept --native and "native" for backwards compatibility, however. The --native option continues to work anywhere it used to previously but is hidden from --help output to discourage new use. Anywhere runner names are accepted, e.g. in config as core.runner or in command-line arguments to `check-setup`, "native" is accepted as well.
tsibley
force-pushed
the
trs/native→ambient
branch
from
b160fc5 to
6b55b97
Compare
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
See commit messages for details.
Related issue(s)
Related to #218.
Testing