Skip to content

Add --version to all con-duct subcommands - #432

Merged
asmacdo merged 4 commits into
mainfrom
copilot/fix-con-duct-version-option
Jun 1, 2026
Merged

Add --version to all con-duct subcommands#432
asmacdo merged 4 commits into
mainfrom
copilot/fix-con-duct-version-option

Conversation

Copilot AI commented May 28, 2026

Copy link
Copy Markdown
Contributor

--version only worked for con-duct (top-level) and con-duct run/duct, but failed on ls, pp, and plot subcommands with "unrecognized arguments: --version".

Changes

  • cli.py: Moves --version from _create_run_parser() into _create_common_parser(), which is already used as a parent for all subcommands — eliminating duplication and giving every subcommand --version for free
  • test_cli.py: Adds a parametrized test covering --version on all four subcommands
$ con-duct ls --version
con-duct ls 0.9.0
$ con-duct pp --version
con-duct pp 0.9.0
$ con-duct plot --version
con-duct plot 0.9.0

Copilot AI linked an issue May 28, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Fix con-duct to support --version command Add --version to all con-duct subcommands May 28, 2026
Copilot AI requested a review from yarikoptic May 28, 2026 13:04
Comment thread test/test_cli.py Outdated
assert re.match(r"con-duct \d+\.\d+\.\d+", output_str)


@pytest.mark.parametrize("subcommand", ["run", "ls", "pp", "plot"])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot I do not see test to test con-duct --version itself. Add parametric empty string here or make commands into the lists and add empty list to concatenate for check_output?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — merged test_con_duct_version and test_con_duct_subcommand_version into a single parametrized test with None for the top-level con-duct --version case (commit 9a7e26a).

Copilot AI requested a review from yarikoptic May 28, 2026 13:10
@yarikoptic
yarikoptic marked this pull request as ready for review May 28, 2026 13:22
Copilot AI review requested due to automatic review settings May 28, 2026 13:22
@yarikoptic yarikoptic added the semver-patch Increment the patch version when merged label May 28, 2026
=== Do not change lines below ===
{
 "chain": [],
 "cmd": "pre-commit install && pre-commit run --all-files || true && ./.update-readme-help.py",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [
  "."
 ],
 "pwd": "."
}
^^^ Do not change lines above ^^^

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes inconsistent CLI behavior by making --version available on every con-duct subcommand (not just the top-level command and run), and adds tests to prevent regressions.

Changes:

  • Add --version to the shared/common argparse parent so all subcommands inherit it.
  • Remove the now-redundant --version definition from the run-specific parser.
  • Add a parametrized CLI test to validate --version works for con-duct, run, ls, pp, and plot.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/con_duct/cli.py Moves --version into the common parent parser so every subcommand supports it.
test/test_cli.py Adds a parametrized test ensuring --version works across all subcommands.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/con_duct/cli.py
Comment on lines +204 to +206
parser.add_argument(
"--version", action="version", version=f"%(prog)s {__version__}"
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into this. The bigger problem with using the common parser would be that it adds --log-level and --quiet to the main command, which wouldn't be that bad except the args dont get passed to the subparsers. So a user could do con-duct --log-level DEBUG run sleep 1 which would use default log-level INFO.

IMO extra machinery just to avoid a 1 line fstring is overkill.

@codecov

codecov Bot commented May 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.92%. Comparing base (f074bb4) to head (b47b0e4).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #432   +/-   ##
=======================================
  Coverage   91.92%   91.92%           
=======================================
  Files          15       15           
  Lines        1127     1127           
  Branches      140      140           
=======================================
  Hits         1036     1036           
  Misses         69       69           
  Partials       22       22           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yarikoptic yarikoptic assigned asmacdo and unassigned yarikoptic May 28, 2026
@asmacdo
asmacdo merged commit cc80272 into main Jun 1, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-patch Increment the patch version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

con-duct needs --version

5 participants