-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Modify toolchain/autoupdate_testdata.py to use scripts/scripts_utils.py #4227
Conversation
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.
Thanks! Sorry about the inconvenience with this.
Is the |
ce45a9e
to
9b464cb
Compare
Thanks for the suggestions, looks much better! |
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.
mypy isn't doing great here, and carbon-lang
is a nuisance for a directory name; can you use run_bazel.py
for this?
9b464cb
to
3c8d410
Compare
I think `paths.parents[2]` is easier to read than `path.parent.parent.parent`, just applying uniformly. I'd noticed this while looking at #4227 Also remove a couple `resolve()` calls that shouldn't be necessary since `__file__` is absolute (elsewhere in the same files, `resolve()` is used to resolve potentially relative paths)
The current
toolchain/autoupdate_testdata.py
script assumes the correct bazel version is already installed. This change usesscripts/scripts_utils.py
to fallback tobazelisk
.