[ci] Add a workflow to auto-remove CICD label#11301
[ci] Add a workflow to auto-remove CICD label#11301stuartmorgan-g wants to merge 3 commits intomainfrom
Conversation
See flutter/flutter#183675. This replicates that script to this repo, until such time as the entire flow is fully centralized.
![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 introduces a new GitHub Actions workflow, .github/remove_cicd.yaml, designed to automatically remove the 'CICD' label from pull requests when new code is pushed. This is a replication of a similar workflow from the flutter/flutter repository.
My review identifies two areas for improvement:
- A logic correction to more accurately determine the push time, ensuring stale labels are reliably removed.
- A suggestion to tighten permissions by removing a permission that appears to be unnecessary, adhering to the principle of least privilege.
.github/remove_cicd.yml
Outdated
| # Get push time (commit date of the head SHA) | ||
| PUSH_TIME=$(gh api repos/${{ github.repository }}/commits/${{ github.event.pull_request.head.sha }} --jq '.commit.committer.date') |
There was a problem hiding this comment.
The current method of determining the "push time" by using the head commit's committer date can be inaccurate. If a commit that was created in the past is pushed to the branch, its committer date will be old, and the script might fail to remove a stale 'CICD' label.
A more reliable and simpler way to get the time of the push for a synchronize event is to use github.event.pull_request.updated_at. This timestamp reflects when the PR was last updated, which corresponds to the push.
# Get PR update time, which corresponds to the push time for a synchronize event
PUSH_TIME='${{ github.event.pull_request.updated_at }}'
There was a problem hiding this comment.
I think this is a valid criticism. The remedy sounds good assuming that is a real piece of information we can grab and it is in the correct iso format.
tarrinneal
left a comment
tarrinneal
left a comment
There was a problem hiding this comment.
gemini's comments look solid to me, but I understand if you want to keep it the same as the other.
|
I think it'll be easiest to maintain if all the copies are identical. |
stuartmorgan-g
added
the
autosubmit
|
autosubmit label was removed for flutter/packages/11301, because - The status or check suite Mac_arm64 ios_platform_tests_shard_3 master has failed. Please fix the issues identified (or deflake) before re-applying this label. |
auto-submit
bot
removed
the
autosubmit
stuartmorgan-g
added
the
autosubmit
|
Agreed on identical. @gaaclarke - what do you think about the comments ? |
auto-submit
bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/packages/11301, because - The status or check suite Mac_arm64 ios_platform_tests_shard_3 master has failed. Please fix the issues identified (or deflake) before re-applying this label. |
responded inline |
4 participants
See flutter/flutter#183675 and flutter/flutter#183905. This replicates that script to this repo, until such time as the entire flow is fully centralized.