Skip to content

Conversation

@njhill
Copy link
Member

@njhill njhill commented Oct 17, 2025

This is a follow-on to PRs #26737 and #26961 to add some clarifying comments that were suggested in review by @russellb.

This is a follow-on to PRs vllm-project#26737 and vllm-project#26961 to add some clarifying comments that were suggested in review.

Signed-off-by: Nick Hill <nhill@redhat.com>
@njhill
Copy link
Member Author

njhill commented Oct 17, 2025

Let's not waste a CI run on this one!

@mergify mergify bot added the v1 label Oct 17, 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 adds clarifying comments to the code, enhancing readability and understanding of the logic behind certain design choices, particularly in the shm_broadcast.py and output.py files. The comments explain the rationale behind the size of the shared memory buffer and the order of operations in the message queue. All changes appear to be well-justified and improve the code's maintainability.

Copy link
Member

@russellb russellb left a comment

Choose a reason for hiding this comment

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

great, thanks!

@russellb russellb enabled auto-merge (squash) October 18, 2025 00:39
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 18, 2025
@vllm-bot vllm-bot merged commit 3b45075 into vllm-project:main Oct 18, 2025
55 of 57 checks passed
@njhill njhill deleted the follow-on-comments branch October 18, 2025 17:47
lywa1998 pushed a commit to lywa1998/vllm that referenced this pull request Oct 20, 2025
adabeyta pushed a commit to adabeyta/vllm that referenced this pull request Oct 20, 2025
albertoperdomo2 pushed a commit to albertoperdomo2/vllm that referenced this pull request Oct 23, 2025
…27130)

Signed-off-by: Nick Hill <nhill@redhat.com>
Signed-off-by: Alberto Perdomo <aperdomo@redhat.com>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
…27130)

Signed-off-by: Nick Hill <nhill@redhat.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
…27130)

Signed-off-by: Nick Hill <nhill@redhat.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
…27130)

Signed-off-by: Nick Hill <nhill@redhat.com>
Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
…27130)

Signed-off-by: Nick Hill <nhill@redhat.com>
Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
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 v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants