-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Misc] Add kv-connector label #25156
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.
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
Conversation
cc @hmellor |
There was a problem hiding this 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 adds a new Mergify rule to automatically label pull requests related to KVConnector
. The new rule is a great addition for better tracking. I've identified a few areas where the file path matching conditions can be improved to be more comprehensive and ensure all relevant changes are captured. My feedback includes suggestions to broaden the scope of the matched paths.
Signed-off-by: NickLucche <nlucches@redhat.com>
/gemini review |
There was a problem hiding this 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 adds a new Mergify rule to automatically label pull requests related to KVConnector
. The logic for identifying relevant file paths seems correct. However, I've found a potential issue with one of the regular expressions for matching pull request titles, which could lead to many false positives. My suggestion aims to make the pattern more specific to avoid incorrect labeling.
Signed-off-by: NickLucche <nlucches@redhat.com>
/gemini review |
There was a problem hiding this 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 adds a Mergify rule to automatically label pull requests related to KVConnector
. The file path and title-based conditions are a good start. I have a couple of suggestions to make the matching more robust and future-proof. Specifically, the regular expressions for matching file paths under disaggregated
directories could be improved to handle different directory structures, and the title matching for P/D
(Prefill/Decode) could be broadened to catch more variations.
Signed-off-by: NickLucche <nlucches@redhat.com>
Thanks @hmellor ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Feel free to action Gemini's comment if you want
Signed-off-by: NickLucche <nlucches@redhat.com>
Signed-off-by: NickLucche <nlucches@redhat.com>
Signed-off-by: NickLucche <nlucches@redhat.com>
Signed-off-by: NickLucche <nlucches@redhat.com>
Signed-off-by: NickLucche <nlucches@redhat.com> Signed-off-by: charlifu <charlifu@amd.com>
Add a label for
KVConnector
related changes so it's easier to track, especially for OOT implementers looking for breaking interface changes.