[Chore] [CODEOWNERS] Expand multiprocess and server reviewer pools#3016
[Chore] [CODEOWNERS] Expand multiprocess and server reviewer pools#3016sammshen wants to merge 2 commits intoLMCache:devfrom
Conversation
Add @sammshen @KuntaiDu @YaoJiayi @royyhuang @deng451e to /lmcache/v1/multiprocess/ and /lmcache/v1/multiprocess/http_server.py to broaden the review pool. Add @sammshen @royyhuang @deng451e to /lmcache/server/ for the same reason. The /lmcache/v1/multiprocess/protocols/observability.py owner list is intentionally unchanged. Signed-off-by: Samuel Shen <slshen@uchciago.edu>
There was a problem hiding this comment.
Code Review
This pull request updates the .github/CODEOWNERS file to expand the owner lists for the multiprocess and server directories. A review comment suggests removing a redundant entry for http_server.py since its ownership now matches its parent directory, which would simplify the configuration.
| /lmcache/v1/multiprocess/ @ApostaC @OasisGit @sammshen @KuntaiDu @YaoJiayi @royyhuang @deng451e | ||
| /lmcache/v1/multiprocess/protocols/observability.py @royyhuang @ApostaC | ||
| /lmcache/v1/multiprocess/http_server.py @royyhuang @ApostaC @OasisGit | ||
| /lmcache/v1/multiprocess/http_server.py @royyhuang @ApostaC @OasisGit @sammshen @KuntaiDu @YaoJiayi @deng451e |
There was a problem hiding this comment.
This entry for http_server.py is now redundant. Since the parent directory /lmcache/v1/multiprocess/ (line 12) now includes the exact same set of owners (@royyhuang, @ApostaC, @OasisGit, @sammshen, @KuntaiDu, @YaoJiayi, and @deng451e), this specific file override no longer changes the reviewer pool and can be removed to simplify the file.
References
- The style guide emphasizes maintainability and technical debt. Redundant configuration entries increase maintenance overhead. (link)
|
this is also a chance to pass auto pass K3 CI for non code changes and it worked :) |
|
|
||
| # Multiprocess | ||
| /lmcache/v1/multiprocess/ @ApostaC @OasisGit | ||
| /lmcache/v1/multiprocess/ @ApostaC @OasisGit @sammshen @KuntaiDu @YaoJiayi @royyhuang @deng451e |
There was a problem hiding this comment.
hi~ @sammshen could you add me and @maobaolong here too? we are learning about MP mode now.
There was a problem hiding this comment.
yihua will need to approve this PR and leave the final call to him
Add @maobaolong @chunxiaozheng to /lmcache/v1/multiprocess/ and /lmcache/v1/multiprocess/http_server.py. Add @oasisgit to /lmcache/server/. /lmcache/v1/multiprocess/protocols/observability.py ownership is intentionally unchanged. Signed-off-by: Samuel Shen <slshen@uchciago.edu>
e7ef87f to
ebbc361
Compare
|
@chunxiaozheng @maobaolong added!! also we decided to remove the CODEOWNERS restriction but you guys will still be pinged on MP PRs :) |
@sammshen thanks! |
Pull request was closed
What this PR does / why we need it:
Special notes for your reviewers:
If applicable:
Note
Low Risk
Low risk: updates only
.github/CODEOWNERSto adjust review assignment, with no runtime code changes.Overview
Expands
.github/CODEOWNERScoverage by adding additional owners for/lmcache/v1/multiprocess/,multiprocess/http_server.py, and/lmcache/server/, broadening the default reviewer pool for multiprocess and server-related changes.Reviewed by Cursor Bugbot for commit ebbc361. Bugbot is set up for automated code reviews on this repo. Configure here.