Repository navigation
ci: add Prow-style triage and review slash commands - #609
Caesarsage wants to merge 2 commits into
Conversation
Adds a triage bot, run by GitHub Actions, that applies Kubernetes-style slash commands from issue and pull request comments. New issues get needs-triage. Contributors and maintainers triage with /triage, /kind, /area, /platform, /good-first-issue and /help, and contributors pick up accepted issues with /assign. Maintainers also have /priority, /milestone, /keep-open, /close duplicate|wontfix|invalid and the /remove-* commands. On pull requests, /lgtm, /approve, /hold, /unhold, /retest, /cc and /uncc are available; review commands only set labels, so merging still needs a code owner review. A hold check fails while do-not-merge/hold is set. The bot lives in hack/triage (parse.js, plan.js, run.js) with all permission rules in the pure plan() function, and is tested with node --test in the new triage-tests workflow. It only checks out hack/triage from the default branch and never runs pull request code. The welcome messages now point first-time contributors to the triage flow. Commands are documented in documentation/triage-commands.md. Refs microcks#603 Signed-off-by: caesarsage <destinyerhabor6@gmail.com>
|
@Harsh4902 @Vaishnav88sk @yada could you take a look at this |
| Thanks and congrats 🎉 for opening your first issue here! Be sure to follow the issue template or please update it accordingly. | ||
|
|
||
|
|
||
| 🏷️ A maintainer will triage it soon. If you'd like to work on it, please wait for the `triage/accepted` label, then comment `/assign`. See the [triage commands](https://github.com/microcks/microcks-cli/blob/master/documentation/triage-commands.md). |
There was a problem hiding this comment.
@Caesarsage we should remove this "/assign" part. As in Microcks we don't follow this practice.
| | `/assign` | Assigns yourself. On issues, only once the issue has `triage/accepted`. | | ||
| | `/unassign` | Unassigns yourself. | |
|
@Caesarsage There are some minor changes which we've to make. Rest looks good to me. |
|
What's the process of assigning oneself to an issue @Harsh4902 |
|
Maintainer doing the assignment might become too much work as issue progresses and not assigned work come back to anyone raising duplicate PRs (though the assignment doesn't entirely prevent this but a contributor know who is working on the issue and who to collaborate with if needed ) |
We usually don't assign issues to a specific person. We mostly follow "first come, first serve" appraoch and whoever first raise the PR with complete solution, we accept it. |
+1 for this point. When I first contributed to the microcks, lau told the same thing to me. 😄 |
Okay, I think people can still assign themselves if they want and it doesn't prevent the first come, first serve PR approach. |
Vaishnav88sk
left a comment
Vaishnav88sk
left a comment
There was a problem hiding this comment.
This is my only concern. other than this It looks very good to me. And yes we can test this system on cli before applying on others.
| | `/remove-lgtm` (`/lgtm cancel`), `/remove-approve` (`/approve cancel`) | Maintainers | Removes `lgtm` or `approved`. | | ||
| | `/hold` | Contributors, maintainers and the author | Adds `do-not-merge/hold`. The `hold` check fails while the label is set. | | ||
| | `/unhold` (`/hold cancel`) | Maintainers | Removes `do-not-merge/hold`. | | ||
| | `/retest` | Maintainers and the author | Re-runs the failed checks on the latest commit. | |
There was a problem hiding this comment.
I think in prow, this /retest is due to it runs the the k8s ecosystem code in the environment and req that infra. And infra can fail, so it is there. But here we dont have such situation, so I am confused about the use of this?
There was a problem hiding this comment.
It's using the same prow idea but entirely different implementation.
The retest here simply rerun the existing test/checks when someone open a PR today.
Microcks doesn't assign issues to contributors: the first complete pull request is accepted. Contributors now get a short explanation instead of being assigned, and the welcome message no longer mentions /assign. Document why /retest exists: re-running checks from the Actions tab needs write access. Signed-off-by: caesarsage <destinyerhabor6@gmail.com>
|
@Harsh4902 I have updated the PR |
Refs #603
Adds Kubernetes-style slash commands for issues and PRs, run by GitHub Actions (no Prow). New issues get
needs-triage. Issues aren't assigned: once an issue hastriage/accepted, the first complete PR is accepted. Commands are listed indocumentation/triage-commands.md.Why: PRs were opened before the issue was agreed on (#597), and contributors opened duplicate PRs (#605, #607).
Permissions
/lgtm, but not on their own items./assignand/unassignare maintainer-only./approve,/priority,/milestoneand the/remove-*commands are maintainer-only./lgtmor/approvetheir own PR.Review commands only set labels, so merging still needs a code owner review.
Security: the bot checks out only
hack/triagefrom the default branch and never runs PR code. Commands hidden in HTML comments are ignored.Beyond #603: this PR also adds the PR review commands,
/priority,/milestoneand aholdcheck. Happy to split these into a separate PR.After merge: I'll create the new
triage/*,priority/*,lgtm,approvedanddo-not-merge/holdlabels.AI disclosure: I wrote this with Claude Code and reviewed and tested all of it. Testing on my fork: Caesarsage#2 and Caesarsage#3.