Revise checking toolchain path #2675
Conversation
sarahmarshy
force-pushed
the
toolchain_path_check
branch
from
cabf057
to
be10711
Compare
| nested_dir=None): | ||
| """ | ||
| Positional args: | ||
| tool_key: the key to index TOOLCHAIN_PATHS |
There was a problem hiding this comment.
Since we are only indexing, should we just pass the list of paths itself?
There was a problem hiding this comment.
Guess it is an argument for clarity/readability. I feel like it looks better this way from each respective toolchain point of view, and this function shows where that path comes from. Just passing in **["ARM", "armcc", 2, "bin"] looks better to me than **[TOOLCHAIN_PATHS["ARM"], "armcc", 2, "bin"] (Though I will admit that neither look great, but I am open to suggestions).
There was a problem hiding this comment.
Actually, it is better to have the key, because this line:
TOOLCHAIN_PATHS[tool_key] = exe
changes the value
sarahmarshy
force-pushed
the
toolchain_path_check
branch
2 times, most recently
from
ceec6d5
to
0210764
Compare
sarahmarshy
force-pushed
the
toolchain_path_check
branch
from
0210764
to
ab92a5a
Compare
|
This looks like a better method of validating the existence of the toolchains. @screamerbg Will this work with the online compiler? |
|
I actually really like this method, as it makes it possible to auto-detect which toolchain to use for a compile when none is specified. |
|
Could we add check_executable as an abstractmethod to the mbedToolchain class too? |
|
Looks great! Thanks, Sarah! |
|
/morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
Outputmbed Build Number: 858 All builds and test passed! |
|
LGTM |
Mbed-cli is responsible for checking toolchain paths for executables that will be used for compilation.
@screamerbg