Skip to content

Conversation

@gmagogsfm
Copy link
Contributor

@gmagogsfm gmagogsfm commented Dec 6, 2025

This error is confusing because -O.xx is too similar to -On which might confuse people. e.g. if a user does -O.2 thinking they're setting optimisation level but then they see an error suggesting they do -cc.2 which also won't work.

More context:
#29991 (comment)

Signed-off-by: Yanan Cao <gmagogsfm@gmail.com>
@gmagogsfm gmagogsfm changed the title Remove confusing -O.xx flag error [Frontend] Remove confusing -O.xx flag error Dec 6, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request removes a confusing error message for the deprecated -O.xx command-line flag by removing its special handling. This allows the standard argument parser to handle invalid values, which is a cleaner approach. The corresponding test is also removed. The changes are correct and improve the user experience by providing less confusing error messages. I have no further comments.

@gmagogsfm
Copy link
Contributor Author

@hmellor Hi, please see this PR for removing the confusing error.

@hmellor hmellor enabled auto-merge (squash) December 6, 2025 11:17
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Dec 6, 2025
@hmellor hmellor merged commit cbedb70 into vllm-project:main Dec 7, 2025
44 checks passed
@gmagogsfm gmagogsfm deleted the remove_O_error branch December 7, 2025 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants