MINOR Remove needs-attention label after review#18366
Conversation
|
Tested on my fork 
|
.github/workflows/pr-reviewed.yml
Outdated
| issue_number: pull.number, | ||
| name: "triage" | ||
| }); | ||
| try { |
There was a problem hiding this comment.
Out of curiosity, why do we have extra logging specifically for removing the needs-attention label? We don't seem to have similar logging for other labels like triage.
There was a problem hiding this comment.
Since the issue search is filtering for label:triage, we know all the found PRs will have that label. We do log something for each of this (on L53)
console.log("Removing 'triage' label from PR " + pull.number + " : " + pull.title);
To be consistent, I thought we should log a message when we remove "needs-attention".
This log line is a bit extraneous. I'll remove it
console.log("PR did not have 'needs-attention' label");
There was a problem hiding this comment.
To ensure both "removeLabel" are executed, should we implement error handling for removing "triage"?
There was a problem hiding this comment.
Hm, this actually makes me realize that triage could be missing. I've modified the workflow to search for both labels separately.
chia7712
left a comment
chia7712
left a comment
There was a problem hiding this comment.
LGTM and one trivial comment
.github/workflows/pr-reviewed.yml
Outdated
| @@ -24,6 +24,9 @@ on: | |||
| jobs: | |||
| remove-triage: | |||
There was a problem hiding this comment.
Given the changes introduced by this PR, we may need to consider renaming this
There was a problem hiding this comment.
That's a good point. I realized the workflow file names are a bit incorrect too, so I rearranged some things. PTAL
|
Run on my fork https://github.com/mumrah/kafka/actions/runs/12677379393 |
Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
2 participants
Follow up from #18147 to also remove the needs-attention label