Skip to content

Fix -y short flag collision between --yaml and --yes (#559) - #561

Open
CodersAcademy006 wants to merge 3 commits into
NVIDIA-NeMo:mainfrom
CodersAcademy006:fix/cli-yaml-yes-short-flag-collision
Open

Fix -y short flag collision between --yaml and --yes (#559)#561
CodersAcademy006 wants to merge 3 commits into
NVIDIA-NeMo:mainfrom
CodersAcademy006:fix/cli-yaml-yes-short-flag-collision

Conversation

@CodersAcademy006

Copy link
Copy Markdown
Contributor

Summary

In RunContext.cli_command (nemo_run/cli/api.py), both --yaml and --yes/--no-confirm claimed the short flag -y, so Click emitted a warning on every invocation. This drops -y from --yaml and keeps it as the conventional alias for --yes.

Closes #559

cc @hemildesai @marcromeyn

RunContext.cli_command bound -y to both --yaml and --yes/--no-confirm,
so Click emitted a warning on every invocation. Drop -y from --yaml,
keeping -y as the conventional alias for --yes.

Closes NVIDIA-NeMo#559

Signed-off-by: Srijan Upadhyay <srjnupadhyay@gmail.com>
@CodersAcademy006

Copy link
Copy Markdown
Contributor Author

@ko3n1g @chtruong814 could one of you take a look at this and at #562? Both have been open since July 14 with no reviewer assigned, and the original cc to @hemildesai and @marcromeyn was not picked up.

This one is a single line in nemo_run/cli/api.py. In RunContext.cli_command both --yaml and --yes/--no-confirm claim the short flag -y, so Click emits a warning on every invocation. It drops -y from --yaml and leaves it as the conventional alias for --yes. Closes #559.

Copy link
Copy Markdown
Contributor

PR #561 — needs attention
Blocking findings: None.
Non-blocking finding: docs/guides/cli.md:349 still advertises -y for --yaml, while nemo_run/cli/api.py:893-900 now reserves -y for --yes. Update the generated/example option table so users are not given a removed alias.
Validation gaps: No regression test asserts warning-free help or verifies that -y selects confirmation bypass. Only DCO is visible.

@ko3n1g

ko3n1g commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

@CodersAcademy006 could you take a look at #561 (comment) please?

@ko3n1g ko3n1g added the waiting-on-customer Waiting on the original author to respond label Aug 7, 2026
@CodersAcademy006

CodersAcademy006 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Hey @ko3n1g Fixed all three blockers. Removed -y from yaml option in api.py line 894, updated docs at 349, added TestShortFlagCollision regression tests to verify -y works for skip_confirmation and yaml has no short flag. Ready to merge.

@svcnvidia-nemo-ci svcnvidia-nemo-ci removed the waiting-on-customer Waiting on the original author to respond label Aug 7, 2026
- Remove -y short flag from --yaml option documentation
- Add TestShortFlagCollision regression test suite with three tests:
  - Verify --yaml flag works independently
  - Verify -y correctly triggers --yes/--no-confirm
  - Ensure no Click warnings in help output

Fixes NVIDIA-NeMo#559
@svcnvidia-nemo-ci svcnvidia-nemo-ci added the waiting-on-maintainers Waiting on maintainers to respond label Aug 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-request waiting-on-maintainers Waiting on maintainers to respond

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RunContext.cli_command assigns the short flag -y to both --yaml and --yes — click warns on every invocation

4 participants