Skip to content

Conversation

@wangxiyuan
Copy link
Collaborator

@wangxiyuan wangxiyuan commented Nov 18, 2025

the patch for spec decode config looks useless now. Let's remove it.

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 patch for SpeculativeConfig, which is described as no longer being useful. This is a good cleanup, as it simplifies the codebase by removing a significant monkey patch, improving maintainability. The changes are straightforward and look correct based on the assumption that the patch is indeed obsolete.

A couple of minor points:

  • There's a typo in the pull request title ('uesless' should be 'useless').
  • For future reference, it would be beneficial to provide more context in the PR description explaining why a change is being made. For instance, explaining why this patch is no longer needed (e.g., 'This functionality is now included in vLLM version X.Y') would be very helpful for other developers understanding the evolution of the codebase.

@github-actions
Copy link

👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:‌‌

  • A PR should do only one thing, smaller PRs enable faster reviews.
  • Every PR should include unit tests and end-to-end tests ‌to ensure it works and is not broken by other future PRs.
  • Write the commit message by fulfilling the PR description to help reviewer and future developers understand.

If CI fails, you can run linting and testing checks locally according Contributing and Testing.

@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

1 similar comment
@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant