[ROCm] Starting to add AMD code reviewers for ROCm components #23496
Conversation
Signed-off-by: Hongxia Yang <[email protected]>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request begins adding AMD code reviewers to ROCm components in the CODEOWNERS file. The changes are a good first step. My review suggests expanding the list of ROCm-related files and directories to ensure more comprehensive code ownership and review coverage, and provides a direct code suggestion to improve the current change.
.github/CODEOWNERS
Outdated
| # ROCm related | ||
| /docker/Dockerfile.rocm @gshtras @hongxiayang | ||
| /docker/Dockerfile.rocm_base @gshtras @hongxiayang | ||
| /vllm/v1/attention/backends/rocm_aiter_fa.py @gshtras @hongxiayang @fsx950223 |
There was a problem hiding this comment.
This is a great start for adding ROCm code owners. To make it more comprehensive, I've suggested adding ownership for the entire csrc/rocm/ directory and used a wildcard for the Dockerfiles. You might also want to consider adding owners for other important files like vllm/platforms/rocm.py, ROCm-related requirements (/requirements/rocm*.txt), and tests (/tests/kernels/**/test_rocm_*.py) in a follow-up or in this PR.
# ROCm related
/docker/Dockerfile.rocm* @gshtras @hongxiayang
/csrc/rocm/ @gshtras @hongxiayang
/vllm/v1/attention/backends/rocm_aiter_fa.py @gshtras @hongxiayang @fsx950223
There was a problem hiding this comment.
nice review comment from the gemini-code-assist bot
There was a problem hiding this comment.
Thank you for the kind words! I'm glad I could provide helpful suggestions.
|
Codeowners must have write access to the repository, @hongxiayang @fsx950223 do not have this. |
Thanks for the comment. However, folks in AMD would like to get notified when certain files are changed in the pull requests, and would like to review the code promptly to avoid problems later. Unfortunately, most of them don't have write access. What are the alternatives you wound recommend? @DarkLight1337 |
|
I suggest having one person (with write access) to receive and forward CODEOWNERS pings. |
|
Yes, @gshtras does have write access as a member of the vLLM committer team. He can forward CODEOWNER notifications to the relevant people. |
Signed-off-by: Hongxia Yang <[email protected]>
Signed-off-by: Hongxia Yang <[email protected]>
|
ok, @DarkLight1337 @hmellor Kindly to merge it when it is good to go. thanks. |
…roject#23496) Signed-off-by: Hongxia Yang <[email protected]> Signed-off-by: tc-mb <[email protected]>
…roject#23496) Signed-off-by: Hongxia Yang <[email protected]>
…roject#23496) Signed-off-by: Hongxia Yang <[email protected]> Signed-off-by: Xiao Yu <[email protected]>
…roject#23496) Signed-off-by: Hongxia Yang <[email protected]> Signed-off-by: Xiao Yu <[email protected]>
…roject#23496) Signed-off-by: Hongxia Yang <[email protected]>
…roject#23496) Signed-off-by: Hongxia Yang <[email protected]>
…roject#23496) Signed-off-by: Hongxia Yang <[email protected]>
…roject#23496) Signed-off-by: Hongxia Yang <[email protected]>
…roject#23496) Signed-off-by: Hongxia Yang <[email protected]> Signed-off-by: Matthew Bonanni <[email protected]>
…roject#23496) Signed-off-by: Hongxia Yang <[email protected]>
…roject#23496) Signed-off-by: Hongxia Yang <[email protected]> Signed-off-by: Shiyan Deng <[email protected]>
…roject#23496) Signed-off-by: Hongxia Yang <[email protected]>
Purpose
Start to add AMD code reviewers for ROCm only components.
AITER folks would like to review aiter related component like: vllm/v1/attention/backends/rocm_aiter_fa.py to reduce the possibility of unvalidated changes which can cause regression or broken of workflows.
Currently there is only one person with write access, and he will need to ping other folks to help code review when needed.
Potentially, to add workflow rule to notify more folks in the future.
Test Plan
Test Result
(Optional) Documentation Update
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.