Skip to content

Conversation

elvischenv
Copy link
Contributor

@elvischenv elvischenv commented Sep 6, 2025

Purpose

Add --force-overwrite option to generate_cmake_presets.py.

Test Plan

Test Result


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

@elvischenv elvischenv requested a review from hmellor as a code owner September 6, 2025 19:43
@mergify mergify bot added the documentation Improvements or additions to documentation label Sep 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 introduces a --force-overwrite option to the generate_cmake_presets.py script, allowing for non-interactive overwriting of existing CMakeUserPresets.json files. The implementation is straightforward and correct, using argparse to handle the new command-line flag. The logic correctly bypasses the user prompt when the flag is present, which is particularly useful for automated environments. The accompanying documentation in docs/contributing/incremental_build.md is also updated clearly and accurately to reflect this new functionality. The changes are well-contained and I found no issues.

@elvischenv elvischenv force-pushed the elvischenv/improve-gen-cmake-script branch from 55122d1 to acc432e Compare September 8, 2025 04:16
@elvischenv elvischenv changed the title [Misc] Add --force-overwrite option to generate_cmake_presets.py [Doc] Add --force-overwrite option to generate_cmake_presets.py Sep 8, 2025
@elvischenv
Copy link
Contributor Author

cc @mgoin for review. Thanks!

@elvischenv elvischenv force-pushed the elvischenv/improve-gen-cmake-script branch from acc432e to ea8af50 Compare September 10, 2025 19:21
Signed-off-by: elvischenv <219235043+elvischenv@users.noreply.github.com>
@elvischenv elvischenv force-pushed the elvischenv/improve-gen-cmake-script branch from ea8af50 to 2229c6c Compare September 11, 2025 22:01
@hmellor
Copy link
Member

hmellor commented Sep 16, 2025

@tlrmchlsmth / @LucasWilkinson could you take a look at this? It should be a quick review.

Copy link
Member

@mgoin mgoin left a comment

Choose a reason for hiding this comment

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

Thanks for this, LGTM!

@simon-mo simon-mo merged commit 3059b9c into vllm-project:main Sep 17, 2025
14 checks passed
@elvischenv elvischenv deleted the elvischenv/improve-gen-cmake-script branch September 17, 2025 02:15
FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
…-project#24375)

Signed-off-by: elvischenv <219235043+elvischenv@users.noreply.github.com>
charlifu pushed a commit to ROCm/vllm that referenced this pull request Sep 25, 2025
…-project#24375)

Signed-off-by: elvischenv <219235043+elvischenv@users.noreply.github.com>
Signed-off-by: charlifu <charlifu@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants