Skip to content

Conversation

keivenchang
Copy link
Contributor

@keivenchang keivenchang commented Oct 11, 2025

Summary

Document unused Docker build arguments in container/build.sh with deprecation notices. These args will be removed in a future PR.

Unused Arguments

Build 1 (base image):

  • FRAMEWORK, VLLM_FRAMEWORK, VERSION, PYTHON_PACKAGE_VERSION, HF_TOKEN, MAX_JOBS

Build 2 (framework image):

  • FRAMEWORK, VLLM_FRAMEWORK, VERSION, PYTHON_PACKAGE_VERSION, HF_TOKEN, NIXL_REF, NIXL_UCX_REF, ENABLE_KVBM

Impact

Documentation only. No functional changes to build process.

Summary by CodeRabbit

  • Documentation
    • Added deprecation notes in the container build process for several unused build arguments, clarifying they will be removed in a future release.
    • Notes are present across relevant build steps to improve visibility for users customizing builds.
    • No changes to build logic, control flow, or error handling; build behavior and outputs remain unchanged.

- Document that FRAMEWORK, VLLM_FRAMEWORK, VERSION, PYTHON_PACKAGE_VERSION,
  and HF_TOKEN are not currently used by any Dockerfile
- Add comments noting these arguments will be removed soon
- Clarify which build args are unused in Build 1 (base image) and Build 2 (framework image)
- No functional changes to build process
Copy link
Contributor

coderabbitai bot commented Oct 11, 2025

Walkthrough

Added deprecation comments in container/build.sh documenting unused build-args to be removed later. No logic, behavior, or control flow changes.

Changes

Cohort / File(s) Summary
Build script comments
container/build.sh
Added deprecation notes for unused build-args (FRAMEWORK, VLLM_FRAMEWORK, VERSION, PYTHON_PACKAGE_VERSION, HF_TOKEN, MAX_JOBS, NIXL_REF, NIXL_UCX_REF, ENABLE_KVBM); no code or behavior changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

I thump my paws at comments neat,
Deprecated flags take backseat.
No scripts were changed, no gears re-spun,
Just notes to mark what’s soon undone.
In burrows of builds, I stash and keep—
Tomorrow’s trims will be swift and deep. 🐇✨

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description does not follow the repository’s template headings—“#### Overview,” “#### Details,” “#### Where should the reviewer start?”, and “#### Related Issues”—and it omits any pointers to review start locations or issue references. Please revise the description to use the required template sections by adding “#### Overview,” “#### Details,” “#### Where should the reviewer start?” with specific files to inspect, and “#### Related Issues” including any issue numbers or noting none.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title accurately summarizes the main change by noting the addition of deprecation notices for unused Docker build arguments and clearly indicates that no code was modified, making it directly related and informative.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 60975b5 and 900a94b.

📒 Files selected for processing (1)
  • container/build.sh (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: trtllm (arm64)
  • GitHub Check: vllm (arm64)
  • GitHub Check: trtllm (amd64)
  • GitHub Check: Build and Test - dynamo
🔇 Additional comments (3)
container/build.sh (3)

594-600: Deprecation note matches intent.
Thanks for documenting the unused args up front; it’ll make the eventual cleanup less surprising for downstream users.


762-769: Clear guidance before Build 1.
Appreciate the reminder of which build-args are effectively no-ops here—helps avoid confusion while the cleanup is pending.


775-783: Helpful context for Build 2.
Callout keeps everyone aligned on the upcoming removal; no concerns from my side.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

3 participants