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

Fix option --is-view env var #1237

Merged
merged 6 commits into from
Mar 5, 2024
Merged

Fix option --is-view env var #1237

merged 6 commits into from
Mar 5, 2024

Conversation

leighmcculloch
Copy link
Member

@leighmcculloch leighmcculloch commented Mar 5, 2024

What

Change the cli --is-view env var to be SOROBAN_INVOKE_VIEW instead of SYSTEM_TEST_VERBOSE_OUTPUT, retaining support for the previous env var for backwards compatibility.

Why

In #1194 when the env var was added it appears we accidentally named it a name that doesn't have anything to do with the option. The env var is actually the name of an env var used in the system test suite, so I'm going to guess that a find all and rename went awry and maybe renamed an env var here that shouldn't have been.

At the moment two env vars are attached to the option, and only the second one appears to be useable in my testing, as the second env parameter overrides the first.

This change is not a breaking change and can be released in a patch release as a bug fix.

The old env var is not advertised in the help output.

Example

Before

      --is-view
          Do not sign and submit transaction
          
          [env: SYSTEM_TEST_VERBOSE_OUTPUT=]

After

      --is-view
          View the result simulating and do not sign and submit transaction
          
          [env: SOROBAN_INVOKE_VIEW=]

@leighmcculloch leighmcculloch requested a review from fnando March 5, 2024 11:13
@leighmcculloch leighmcculloch marked this pull request as ready for review March 5, 2024 11:14
@leighmcculloch leighmcculloch enabled auto-merge (squash) March 5, 2024 12:18
@leighmcculloch leighmcculloch requested a review from 2opremio March 5, 2024 12:18
@leighmcculloch leighmcculloch merged commit d2759c2 into main Mar 5, 2024
14 checks passed
@leighmcculloch leighmcculloch deleted the cli-opt-is-view branch March 5, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants