Skip to content
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

[v9] Fix broken version check in tbot's tshwrap (#13034) #13037

Merged
merged 3 commits into from
May 31, 2022

Conversation

timothyb89
Copy link
Contributor

Backport of #13034 for branch/v9.


tshwrap performs a tsh version check to ensure it has the functionality we need. Unfortunately, during a final refactoring before merging, we changed the function signature of capture() to require an explicit path to a tsh binary but in a way that was unfortunately not caught by the compiler. The previous syntax meant we just tried to execute the first argument, i.e. version, which is never what we want.

This PR correctly passes the tsh path to capture() to fix the version check.

Verified

This commit was signed with the committer’s verified signature.
sdispater Sébastien Eustace
`tshwrap` performs a tsh version check to ensure it has the
functionality we need. Unfortunately, during a final refactoring
before merging, we changed the function signature of `capture()` to
require an explicit path to a `tsh` binary but in a way that was
unfortunately not caught by the compiler. The previous syntax meant
we just tried to execute the first argument, i.e. `version`, which
is never what we want.

This PR correctly passes the tsh path to `capture()` to fix the
version check.
@r0mant r0mant enabled auto-merge (squash) May 31, 2022 18:04
@r0mant r0mant merged commit b3597d1 into branch/v9 May 31, 2022
@r0mant r0mant deleted the timothyb89/v9/fix-tbot-wrap-version branch May 31, 2022 19:59
@webvictim webvictim mentioned this pull request Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants