[Core] Remove forced None assignment for deprecated PassConfig flags #29994
Conversation
Signed-off-by: arpitkh101 <[email protected]>
arpitkh101
requested review from
ProExpertProg,
WoosukKwon,
hmellor,
houseroad,
mgoin,
robertgshaw2-redhat,
tlrmchlsmth,
yewentao256 and
youkaichao
as code owners
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the lines that force deprecated PassConfig flags to None after their values have been processed. While the intention is to reduce confusion for users inspecting the configuration, this change removes a safety measure that prevents accidental use of deprecated flags elsewhere in the code. This increases the risk of future bugs and inconsistencies. I've left a comment explaining why the original behavior is safer and should be retained.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…n test Signed-off-by: arpitkh101 <[email protected]>
hmellor
left a comment
hmellor
left a comment
There was a problem hiding this comment.
LGTM!
That's a nice improvement to the hash behaviour
github-actions
bot
added
the
ready
ProExpertProg
left a comment
ProExpertProg
left a comment
There was a problem hiding this comment.
LGTM apart from one nit
Signed-off-by: arpitkh101 <[email protected]>
…llm-project#29994) Signed-off-by: arpitkh101 <[email protected]> Co-authored-by: Cyrus Leung <[email protected]>
…llm-project#29994) Signed-off-by: arpitkh101 <[email protected]> Co-authored-by: Cyrus Leung <[email protected]> Signed-off-by: Xingyu Liu <[email protected]>
4 participants
This PR removes forced None assignment for deprecated PassConfig flags, as discussed in #29646. Forcing deprecated flags to None doesn't prevent their use and can be confusing.