Skip to content

Add @tjtanaa to codeowner for ROCm and multi-modal #28360

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youkaichao merged 3 commits into from
Nov 10, 2025
+16 −6
Merged

Add @tjtanaa to codeowner for ROCm and multi-modal #28360

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8982d79
add @tjtanaa to code owner
tjtanaa Nov 9, 2025
57def95
update the codeowner syntax
tjtanaa Nov 9, 2025
1a6a325
handles possible future rocm files
tjtanaa Nov 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions .github/CODEOWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
/vllm/model_executor/layers/quantization @mgoin @robertgshaw2-redhat @tlrmchlsmth @yewentao256 @pavanimajety
/vllm/model_executor/layers/mamba @tdoublep
/vllm/model_executor/model_loader @22quinn
/vllm/multimodal @DarkLight1337 @ywang96 @NickLucche
/vllm/multimodal @DarkLight1337 @ywang96 @NickLucche @tjtanaa
/vllm/vllm_flash_attn @LucasWilkinson
/vllm/lora @jeejeelee
/vllm/reasoning @aarnphm @chaunceyjiang
Expand Down Expand Up @@ -105,11 +105,15 @@ mkdocs.yaml @hmellor
/vllm/attention/ops/triton_unified_attention.py @tdoublep

# ROCm related: specify owner with write access to notify AMD folks for careful code review
/docker/Dockerfile.rocm* @gshtras
/vllm/v1/attention/backends/rocm*.py @gshtras
/vllm/v1/attention/backends/mla/rocm*.py @gshtras
/vllm/attention/ops/rocm*.py @gshtras
/vllm/model_executor/layers/fused_moe/rocm*.py @gshtras
/docker/Dockerfile.rocm* @gshtras @tjtanaa
/vllm/v1/attention/backends/rocm*.py @gshtras @tjtanaa
/vllm/v1/attention/backends/mla/rocm*.py @gshtras @tjtanaa
/vllm/attention/ops/rocm*.py @gshtras @tjtanaa
/vllm/model_executor/layers/fused_moe/rocm*.py @gshtras @tjtanaa
Comment on lines +110 to +113
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 9, 2025

Choose a reason for hiding this comment

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

high

To better align with the PR's goal of covering all ROCm-related files and to simplify the CODEOWNERS file, consider using a single glob pattern for all rocm files under the /vllm directory. This would also ensure future ROCm-related files are automatically covered, which seems to be part of your intent as stated in the PR description. The current changes only add ownership to existing specific paths.

/vllm/**/*rocm* @gshtras @tjtanaa

/csrc/rocm @gshtras @tjtanaa
*rocm* @tjtanaa
*quark* @tjtanaa
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Nov 9, 2025

Choose a reason for hiding this comment

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

P1 Badge Keep existing ROCm reviewers when adding wildcard

In CODEOWNERS the last matching rule wins, so the newly added *rocm* @tjtanaa wildcard overrides the more specific ROCm patterns above that list @gshtras as a reviewer. Any file whose name contains rocm will now request only @tjtanaa, defeating the preceding comment about notifying AMD folks and silently removing @gshtras from review for every ROCm path. Either the wildcard should include the previous owners or it should appear before the specific entries to avoid dropping current coverage.

Useful? React with 👍 / 👎.

*aiter* @tjtanaa
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 9, 2025

Choose a reason for hiding this comment

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

critical

These broad wildcard patterns are problematic because they override the more specific ownership rules defined on lines 108-113. According to GitHub's documentation, the last matching pattern in the CODEOWNERS file determines the owners.

For example, the rule *rocm* @tjtanaa on line 114 will make @tjtanaa the sole owner for any path containing rocm, including those on lines 108-113. This removes @gshtras as a codeowner for those files, which is likely not the intended behavior.

The same issue applies to *aiter*, which would override ownership for files like vllm/model_executor/layers/fused_moe/rocm_aiter_fused_moe.py.

To fix this, please remove these broad patterns. If you need to add @tjtanaa as a codeowner for other directories (like requirements, tests, docs as mentioned in the PR description), you should add more specific rules for them, for example:

/requirements/*rocm* @tjtanaa
/tests/**/*rocm* @tjtanaa
/docs/**/*rocm* @tjtanaa

/vllm/**/*quark* @tjtanaa
/tests/**/*quark* @tjtanaa
/docs/**/*quark* @tjtanaa

/vllm/**/*aiter* @tjtanaa
/tests/**/*aiter* @tjtanaa

For now, I'll suggest removing these lines to fix the immediate ownership issue. You can then add the more specific paths in a follow-up.


# TPU
/vllm/v1/worker/tpu* @NickLucche
Expand Down
Loading