-
Notifications
You must be signed in to change notification settings - Fork 14k
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
KAFKA-17542: Use actions/labeler for automatic PR labeling #17208
Conversation
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.
raft
and metadata
aren't labels we currently have. You can check the existing ones here: https://github.com/apache/kafka/labels
Hi @mumrah , thanks for your review and info! |
@TaiJuWu thanks for this patch I believe it will be an important feature to our PR flow. I list some brainstorming below. PTAL
|
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.
Left some additional comments, @TaiJuWu.
I like @chia7712's suggestions. For 3 and 4, we may probably will need to do something more complex than simply using the labeler action. For example,
- add test label if only test code gets changed
will require getting the diff of the PR and checking the file paths. This will require a script of some kind.
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.
Yes, you are right, but the action/labeler does not support scripts. I am still investigating how to achieve this goal. |
We can add an additional step to evaluate the diff and determine what labels to apply. We can use octokit or |
Ok. I am happy to handle this. |
Here is an example of using the https://github.com/apache/kafka/blob/trunk/.github/actions/gh-api-update-status/action.yml It would be nice to have a reusable action similar to this for adding labels |
Got it. Thanks a lot. |
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.
@TaiJuWu thanks for this patch!
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.
edit: see below
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.
Actually found a few issues remaining. Can you address these and then I'll merge. We don't need this to be perfect -- just a good starting point 😄
People focusing on specific project areas will be free to modify this config to auto-label things as they see fit.
Hi @mumrah , all comments are addressed, thanks for 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.
last nit 😄
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!
KIP-932: | ||
- changed-files: | ||
- any-glob-to-any-file: | ||
- 'share/**' | ||
- 'share-coordinator/**' |
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.
This is great thank you, I was manually tagging them @TaiJuWu 👍
Jira: https://issues.apache.org/jira/browse/KAFKA-17542
test on TaiJuWu#5
Committer Checklist (excluded from commit message)