From 91b60ea90025f5b113dbfaeeee1176af49898ff3 Mon Sep 17 00:00:00 2001 From: wuhuizuo Date: Mon, 5 Jun 2023 18:58:42 +0800 Subject: [PATCH 1/9] docs: add new workflow documents Signed-off-by: wuhuizuo --- docs/_sidebar.md | 2 + docs/en/_sidebar.md | 3 +- docs/en/plugins/label-blocker.md | 14 +++--- docs/en/workflows/pr-old.md | 70 ++++++++++++++++++++++++++++++ docs/en/workflows/pr.md | 73 ++++++++------------------------ docs/plugins/label-blocker.md | 14 +++--- docs/workflows/pr-old.md | 64 ++++++++++++++++++++++++++++ docs/workflows/pr.md | 55 ++++++------------------ 8 files changed, 182 insertions(+), 113 deletions(-) create mode 100644 docs/en/workflows/pr-old.md create mode 100644 docs/workflows/pr-old.md diff --git a/docs/_sidebar.md b/docs/_sidebar.md index a491c9027..7d3644cf3 100644 --- a/docs/_sidebar.md +++ b/docs/_sidebar.md @@ -29,3 +29,5 @@ - [autobump](tools/autobump.md) - [工作流](workflows.md) - [PR 工作流](workflows/pr.md) + - [PR 工作流(老)](workflows/pr-old.md) + diff --git a/docs/en/_sidebar.md b/docs/en/_sidebar.md index ffaa4473e..8a6fbd9f9 100644 --- a/docs/en/_sidebar.md +++ b/docs/en/_sidebar.md @@ -28,4 +28,5 @@ - [label_sync](en/tools/label_sync.md) - [autobump](en/tools/autobump.md) - [Workflows](en/workflows.md) - - [PR workflow](en/workflows/pr.md) \ No newline at end of file + - [PR workflow](en/workflows/pr.md) + - [PR workflow(old)](en/workflows/pr-old.md) diff --git a/docs/en/plugins/label-blocker.md b/docs/en/plugins/label-blocker.md index 47874461a..6965bf107 100644 --- a/docs/en/plugins/label-blocker.md +++ b/docs/en/plugins/label-blocker.md @@ -2,7 +2,7 @@ ## Design Background -Throughout the [PR collaboration process](en/workflows/pr.md), some labels are recognized and used by the bot, for example, to determine if a PR can be merged based on whether it has a `status/can-merge` label. For these sensitive labels, we don't want them to be added or removed at will, so we designed the ti-community-label-blocker plugin to help us control the permissions for adding or removing such labels. +Throughout the [PR collaboration process](en/workflows/pr-old.md), some labels are recognized and used by the bot, for example, to determine if a PR can be merged based on whether it has a `status/can-merge` label. For these sensitive labels, we don't want them to be added or removed at will, so we designed the ti-community-label-blocker plugin to help us control the permissions for adding or removing such labels. ## Permission design @@ -23,13 +23,13 @@ For trusted users, i.e. trusted Github users or members of a trusted Github team ### BlockLabel -| Parameter Name | Type | Description | -| -------------- | -------- | ------------------------------------------------------------------ | -| regex | string | Regular expressions for matching label | +| Parameter Name | Type | Description | +| -------------- | -------- | ------------------------------------------------------------------------ | +| regex | string | Regular expressions for matching label | | actions | []string | Matching action type, can fill in `labeled` or `unlabeled`, at least one | -| trusted_teams | []string | Trusted GitHub teams | -| trusted_users | []string | Trusted GitHub users | -| message | string | Feedback hints to the user, empty means no hints | +| trusted_teams | []string | Trusted GitHub teams | +| trusted_users | []string | Trusted GitHub users | +| message | string | Feedback hints to the user, empty means no hints | For example: diff --git a/docs/en/workflows/pr-old.md b/docs/en/workflows/pr-old.md new file mode 100644 index 000000000..d33a8076f --- /dev/null +++ b/docs/en/workflows/pr-old.md @@ -0,0 +1,70 @@ +# PR workflow + +## Design Background + +The collaboration process of PR has changed a bit after the introduction of Tide, but the basic framework has been kept. +The main adjustment is in the PR merge phase, where instead of a one-time `/merge` command that triggers a bot to run tests and merge code, `/merge` is now only responsible for tagging `status/can-merge`. +When the PR labels are satisfied and all tests pass, the PR will be merged automatically without human intervention. + +**⚠️ Note: Please read the [Tide](en/components/tide.md), [ti-community-lgtm](en/plugins/lgtm.md), [ti-community-tars](en/plugins/tars.md), [ti-community-blunderbuss](en/plugins/blunderbuss.md), and [ti-community-merge](en/plugins/merge.md) chapters carefully before reading the following.** + +## PR Collaboration Process + +- Author Submit PR +- **Phase I:** Automatically request reviewers to PR ([ti-community-blunderbuss](en/plugins/blunderbuss.md) provide support) + - Automatically request reviewers based on the sig to which the current PR belongs + - Randomly select multiple reviewers based on ti-community-blunderbuss configuration +- **Phase II:** reviewers review code ([ti-community-lgtm](en/plugins/lgtm.md) provide support) + - reviewer will look at the quality of the code, correctness, engineering considerations, etc. + - If the reviewer finds no problems with the code, the reviewer will approve the changes by GitHub Approve; if the reviewer later finds that there are still problems with the code, they can cancel the changes by GitHub Request Changes + - Once the reviewer uses the above command, the bot ti-chi-bot will automatically add or remove the lgtm-related labels +- **Phase III:** committers review code ([ti-community-merge](en/plugins/merge.md) provide support) + - committer to review the PR again, looking at dependencies with other features, forward/backward compatibility, etc. + - If the committer thinks there will be no problems with the code changes, the committer will use `/merge` to agree to the merge; if the committer later thinks there are still problems with the code, it can cancel the merge by using `/merge cancel` + - Once the committer uses the above command, the bot ti-chi-bot will automatically add or remove the `status/can-merge` label +- **Phase IV:** Auto-merge PR ([Tide](en/components/tide.md) provide support) + - If all of the following requirements are met + - All labels requested to already exist (e.g. status/can-merge) + - No labels that prevent merging (e.g. do-not-merge/hold, needs-rebase) + - If any of the following requirements are met + - No CI testing for current PR + - All tests triggered by the current PR have passed + - After the above requirements are met, Tide automatically merges the PR + +### Collaboration Flow Chart +- [Collaboration flowchart for PR requiring two lgtms(In Chinese)](https://viewer.diagrams.net/?highlight=0000ff&edit=_blank&layers=1&nav=1&title=pr-workflow.drawio#R7Vxdc5s4FP01emwGEB%2FiERw77W62022zm%2B3TDrEVmxaDF%2BM47q9fCYQBIWM5YOO6nslMLCGE4N5zdO%2BRAMDB%2FPUu9hazP6IJDoCmTF4BvAWapuqaSf7Rmk1WY2koq5jG%2FoQ1Kiq%2B%2BD8wq1RY7cqf4GWlYRJFQeIvqpXjKAzxOKnUeXEcravNnqOgetWFN8W1ii9jL6jXPvqTZMZqVdMuDrzH%2FnTGLo00Kzsw9%2FLG7E6WM28SrUtVcAjgII6iJPs1fx3ggD68%2FLk8ftg8Bvffzbvf%2Flz%2B5%2F3l%2Fv7w8e93WWejQ07Z3kKMw6TbrrWs6xcvWLHnxe412eQPMI5W4QTTThQA3VkyD8hPlfz8hpNkwwzurZKIVEVxMoumUegF91G0YO2eozBhzVRaxuHEoYYl5XHgLZf%2B%2BGHmh9mBkR%2Fk3ZMSOwuR0jKJo%2B9bC9LjW3PQYQXeEw5cb%2Fx9mg53EAVRTA6FUYhpVxPiEuyOiiEOi1pysSTe%2FEM7uzHy4lfWd1q4fa2UNqwkaRlmwWW0ise4oR1kAPHiKWb93ePhhxfzk%2Fq8mv47%2FaZ9%2BLieh8zSCr2vkpczu9%2FhaI7JIEmDGAde4r9UoeAxRE237banfop8ch%2BawtC%2FdX2GfdVSql1kA2VnFb5HfpSGUVSlHnmAd8K6dw4RcHXgqGBoATQAzpDWIAUgAwxNgEbAVsBQB64DHB1opjcnXuiGT8tFaizl0%2BcG%2F6ZetZ75Cf6y8FIzrQklVn2%2B7MvkqblT6sDMF%2Fa46GGu8oLjBL82Gjc%2FalaNhFhxXWK73I6zEtHpym53qBjyUKshgdVsaiPbSc03Asi6skyPLJNPzXtpRpekGeaJ75QbrcoX7EJvpaG8SfT8vMQVhtlLVdA8LVXZAqc3gU0YypHw%2Fv2ucwn4yL1frfh%2BAYUz837ZSZZ5oHKjGAidzOF1jvY1Q87hidG9TanZgjZY7o4BIAcszVIax8W3V%2B1Ke%2FIjG0Gn6MttWoJfjF98vCZ1KQ4RcEZgaNDowDFlpqNZNH9aLfcHBBX%2FpxgbeXM%2FoPf%2BHgcvOPHHniBs8AJ%2FGlLkEZfGsRhY5JJ%2BOCUlsyg9pHAmvHzEcMLi7A0F8YQiiCdQB%2FGEEIf1lORQuhQSooA4S3ZtN1VznPE29moipTJ5NSCiRUbQymR1OH5O4Uh8nQLSAs6A%2FtFAPY3eCSAJRF2TYpWE9K6RjjQDcGOguAedO5xf1h7SMfcWNKcIuoVPXK09qFOA5NVPShghpa%2BlIwVCaKFHgOxu1xc%2BzHq02IP5jm0KXdIURp%2Bm0HcF7q6TBgw2cBBTFGiNCVwLuC49hBzgKKnRvISECtro%2Fu6hDsOz5Cto9M1X6DqpcwCQAIrVJ1AM%2BTndodLcAXP6HlRU7X4qjOiiwFeEET7x6m5O72dSP1OQWJIg6TfyVQUrKnL6zz0VT6qmqKWJ1OtJRhk47MDcn0wyq%2BOl%2F8N7SvujxmBZPenccIFx2wQbtvDGTgZbIa1suAb3lFcDmZrfvTbyBhWhycN%2BORHhCFzKiQiG0reIcNVnj6bP5llgnveVzuotC5SdLHakHoWkqyHzWLRVX8PgUcNH39lt1yTdekeIC2UkF0PaasP5gGW1Yd0wOGC304bF05Qgw7uGVVy4JBNXwZZxldzCnaXI%2BWpn7lHXKtsIAHXfOksBwNT6FgDUX0MDU2W39Ki9qmBqfavNIJrP%2FSRh6b1Bc3i67cakPu%2FW5ZuzzOFNu3c3FwSel%2BjmshKWivp0c01E9heanduNSCHJOYuLzjcfV%2BuSo4CIrpl5m8zcEq1cijJzfutndwxZl12uqfkFp%2BbyU8WOhKOn5JzLqVF%2BHwcn5zaHQHik5BxyA1aMxnHx7S21241b4glZIJdfYniEJH2%2B9Ub%2BdlQs2rr9xlx47IXv5pjcbn3yPceEGMnu4OHR2l1savbh%2BYUsddjMcmzEbN8V24sYvU%2FEQAF%2FHRi8%2FHSZReaoTet%2B%2BhEn5q5mnhzHVxX4ILjB06jAqsIrMkeWgWG%2BUfrylQG4Y4NEoQxUTJGvpbW0sFo1cL4F8vhA10SWNel%2BJORSyzojYBtXFaGVimDLviRwNBVB62VD4dFpWXYL7ZFeANa5XBXx%2BzM6ylV13p%2FM5ly11r6aq%2B6dXuyT5LaircXNry8jYJtpNnVLX2WmidYtjSWLNjblIXKU0hXhMCUlMCulKyN9%2B1ltCj%2FrtMdj5ixWcaDOs4t1Y9T4xRbQi8FpMt3Ri91LtHiG71fIJ2a9ShlQ78VgZxreQ9lVaNh2FVoyvFdPvMsDij4l0aBsGZRb7dtSfJiGhYSg6SGbbmv%2FKWStIo%2FqTdeC%2FehaOXUesPJxrloYbLti0jYKFC8MdB0EqipPErA5CuRPOMmKBdy5XInsUlhm0cAL6aKP0lyEhtCsASo3qqZXZQR2xTMWBK%2BvpL2Fc%2FS2r9tIRgzbijezTucOs%2BOdAouuhBVMcKEEsGMfV7EIoOgVA%2BafcThjAhBtN6rLgm24%2FqoY1jOA3iTDPKwp2Zt%2BHk45KNY%2FxTfjTpAmCL%2Fy0tFGWVIsPpKZgbX41Cgc%2Fg8%3D) + - If a new commit is found in the process, **it will keep the lgtm related label, but will automatically remove the `status/can-merge` label** + - When some tests fail, **just trigger the failed test** + +## Recommended configuration items + +### Recommend using Squash mode to merge code + +We recommend using GitHub's Squash mode for merging, because it's a tradition in the TiDB community to create a lot of commits in PR and then automatically Squash them through GitHub when merging. +Our ti-community-merge is currently designed to work in Squash mode, **if you don't use Squash mode, then you are responsible for your own rebase or squash PR in PR, which will disable our ability to store commit hash (see Q&A for details) and eventually cause status/can-merge to be automatically removed due to a new commit**. +So we strongly recommend that you use Squash mode for collaboration. + +### It is recommended to turn on the `Require branches to be up to date before merging` branch protection option for small repositories + +Turning this feature on solves the problem we mentioned in the Tide introduction: + +> - PR1: Rename bifurcate() to bifurcateCrab() +> - PR2: Invoke bifurcate() +> +> In this case, both PRs will be tested with the current master as the base branch, and both PRs will pass. However, once PR1 is merged into the master branch first, and the second PR is merged (because the test also passes), it causes a master error that `bifurcate` is not found. +> + +This feature requires the PR to merge the current master to the PR before merging, so that our PR uses the latest master branch as the Base test. However, turning this feature on has two effects: +1. PR cannot be merged automatically until the latest master is merged into PR +2. PR merge current master to PR causes `status/can-merge` label to disappear + +**The first problem is solved by using [ti-community-tars](en/plugins/tars.md) to automatically update. The second problem is that we can identify the committer of the commit that updates master to PR using the GitHub update button as `web-flow`, so we can determine if we trust that commit based on the committer.** + +## Q&A + +### Why does my own rebase or squash commit cause `status/can-merge` to be removed? + +**Because we are currently storing the hash of the last commit in your PR when tagged with `status/can-merge`**. +When you rebase the PR, the entire hash will change, so it will be untagged automatically. +When you squash the PR yourself, because we store the hash of the last commit and not the hash of the first commit, this will still result in automatic remove label. + diff --git a/docs/en/workflows/pr.md b/docs/en/workflows/pr.md index d33a8076f..0ace49f89 100644 --- a/docs/en/workflows/pr.md +++ b/docs/en/workflows/pr.md @@ -1,70 +1,33 @@ -# PR workflow +# PR Workflow -## Design Background +## Design Background -The collaboration process of PR has changed a bit after the introduction of Tide, but the basic framework has been kept. -The main adjustment is in the PR merge phase, where instead of a one-time `/merge` command that triggers a bot to run tests and merge code, `/merge` is now only responsible for tagging `status/can-merge`. -When the PR labels are satisfied and all tests pass, the PR will be merged automatically without human intervention. +We use prow's native lgtm + approve related plug-ins to drive the process, but we have added the optional configuration of multi-person lgtm on this basis. -**⚠️ Note: Please read the [Tide](en/components/tide.md), [ti-community-lgtm](en/plugins/lgtm.md), [ti-community-tars](en/plugins/tars.md), [ti-community-blunderbuss](en/plugins/blunderbuss.md), and [ti-community-merge](en/plugins/merge.md) chapters carefully before reading the following.** +## Most of the PR collaboration process -## PR Collaboration Process +It's the same as the upstream prow native process, it is recommended to read [here](https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#code-review -using-owners-files). -- Author Submit PR -- **Phase I:** Automatically request reviewers to PR ([ti-community-blunderbuss](en/plugins/blunderbuss.md) provide support) - - Automatically request reviewers based on the sig to which the current PR belongs - - Randomly select multiple reviewers based on ti-community-blunderbuss configuration -- **Phase II:** reviewers review code ([ti-community-lgtm](en/plugins/lgtm.md) provide support) - - reviewer will look at the quality of the code, correctness, engineering considerations, etc. - - If the reviewer finds no problems with the code, the reviewer will approve the changes by GitHub Approve; if the reviewer later finds that there are still problems with the code, they can cancel the changes by GitHub Request Changes - - Once the reviewer uses the above command, the bot ti-chi-bot will automatically add or remove the lgtm-related labels -- **Phase III:** committers review code ([ti-community-merge](en/plugins/merge.md) provide support) - - committer to review the PR again, looking at dependencies with other features, forward/backward compatibility, etc. - - If the committer thinks there will be no problems with the code changes, the committer will use `/merge` to agree to the merge; if the committer later thinks there are still problems with the code, it can cancel the merge by using `/merge cancel` - - Once the committer uses the above command, the bot ti-chi-bot will automatically add or remove the `status/can-merge` label -- **Phase IV:** Auto-merge PR ([Tide](en/components/tide.md) provide support) - - If all of the following requirements are met - - All labels requested to already exist (e.g. status/can-merge) - - No labels that prevent merging (e.g. do-not-merge/hold, needs-rebase) - - If any of the following requirements are met - - No CI testing for current PR - - All tests triggered by the current PR have passed - - After the above requirements are met, Tide automatically merges the PR +The following is the part of the difference relative to the original upstream: -### Collaboration Flow Chart -- [Collaboration flowchart for PR requiring two lgtms(In Chinese)](https://viewer.diagrams.net/?highlight=0000ff&edit=_blank&layers=1&nav=1&title=pr-workflow.drawio#R7Vxdc5s4FP01emwGEB%2FiERw77W62022zm%2B3TDrEVmxaDF%2BM47q9fCYQBIWM5YOO6nslMLCGE4N5zdO%2BRAMDB%2FPUu9hazP6IJDoCmTF4BvAWapuqaSf7Rmk1WY2koq5jG%2FoQ1Kiq%2B%2BD8wq1RY7cqf4GWlYRJFQeIvqpXjKAzxOKnUeXEcravNnqOgetWFN8W1ii9jL6jXPvqTZMZqVdMuDrzH%2FnTGLo00Kzsw9%2FLG7E6WM28SrUtVcAjgII6iJPs1fx3ggD68%2FLk8ftg8Bvffzbvf%2Flz%2B5%2F3l%2Fv7w8e93WWejQ07Z3kKMw6TbrrWs6xcvWLHnxe412eQPMI5W4QTTThQA3VkyD8hPlfz8hpNkwwzurZKIVEVxMoumUegF91G0YO2eozBhzVRaxuHEoYYl5XHgLZf%2B%2BGHmh9mBkR%2Fk3ZMSOwuR0jKJo%2B9bC9LjW3PQYQXeEw5cb%2Fx9mg53EAVRTA6FUYhpVxPiEuyOiiEOi1pysSTe%2FEM7uzHy4lfWd1q4fa2UNqwkaRlmwWW0ise4oR1kAPHiKWb93ePhhxfzk%2Fq8mv47%2FaZ9%2BLieh8zSCr2vkpczu9%2FhaI7JIEmDGAde4r9UoeAxRE237banfop8ch%2BawtC%2FdX2GfdVSql1kA2VnFb5HfpSGUVSlHnmAd8K6dw4RcHXgqGBoATQAzpDWIAUgAwxNgEbAVsBQB64DHB1opjcnXuiGT8tFaizl0%2BcG%2F6ZetZ75Cf6y8FIzrQklVn2%2B7MvkqblT6sDMF%2Fa46GGu8oLjBL82Gjc%2FalaNhFhxXWK73I6zEtHpym53qBjyUKshgdVsaiPbSc03Asi6skyPLJNPzXtpRpekGeaJ75QbrcoX7EJvpaG8SfT8vMQVhtlLVdA8LVXZAqc3gU0YypHw%2Fv2ucwn4yL1frfh%2BAYUz837ZSZZ5oHKjGAidzOF1jvY1Q87hidG9TanZgjZY7o4BIAcszVIax8W3V%2B1Ke%2FIjG0Gn6MttWoJfjF98vCZ1KQ4RcEZgaNDowDFlpqNZNH9aLfcHBBX%2FpxgbeXM%2FoPf%2BHgcvOPHHniBs8AJ%2FGlLkEZfGsRhY5JJ%2BOCUlsyg9pHAmvHzEcMLi7A0F8YQiiCdQB%2FGEEIf1lORQuhQSooA4S3ZtN1VznPE29moipTJ5NSCiRUbQymR1OH5O4Uh8nQLSAs6A%2FtFAPY3eCSAJRF2TYpWE9K6RjjQDcGOguAedO5xf1h7SMfcWNKcIuoVPXK09qFOA5NVPShghpa%2BlIwVCaKFHgOxu1xc%2BzHq02IP5jm0KXdIURp%2Bm0HcF7q6TBgw2cBBTFGiNCVwLuC49hBzgKKnRvISECtro%2Fu6hDsOz5Cto9M1X6DqpcwCQAIrVJ1AM%2BTndodLcAXP6HlRU7X4qjOiiwFeEET7x6m5O72dSP1OQWJIg6TfyVQUrKnL6zz0VT6qmqKWJ1OtJRhk47MDcn0wyq%2BOl%2F8N7SvujxmBZPenccIFx2wQbtvDGTgZbIa1suAb3lFcDmZrfvTbyBhWhycN%2BORHhCFzKiQiG0reIcNVnj6bP5llgnveVzuotC5SdLHakHoWkqyHzWLRVX8PgUcNH39lt1yTdekeIC2UkF0PaasP5gGW1Yd0wOGC304bF05Qgw7uGVVy4JBNXwZZxldzCnaXI%2BWpn7lHXKtsIAHXfOksBwNT6FgDUX0MDU2W39Ki9qmBqfavNIJrP%2FSRh6b1Bc3i67cakPu%2FW5ZuzzOFNu3c3FwSel%2BjmshKWivp0c01E9heanduNSCHJOYuLzjcfV%2BuSo4CIrpl5m8zcEq1cijJzfutndwxZl12uqfkFp%2BbyU8WOhKOn5JzLqVF%2BHwcn5zaHQHik5BxyA1aMxnHx7S21241b4glZIJdfYniEJH2%2B9Ub%2BdlQs2rr9xlx47IXv5pjcbn3yPceEGMnu4OHR2l1savbh%2BYUsddjMcmzEbN8V24sYvU%2FEQAF%2FHRi8%2FHSZReaoTet%2B%2BhEn5q5mnhzHVxX4ILjB06jAqsIrMkeWgWG%2BUfrylQG4Y4NEoQxUTJGvpbW0sFo1cL4F8vhA10SWNel%2BJORSyzojYBtXFaGVimDLviRwNBVB62VD4dFpWXYL7ZFeANa5XBXx%2BzM6ylV13p%2FM5ly11r6aq%2B6dXuyT5LaircXNry8jYJtpNnVLX2WmidYtjSWLNjblIXKU0hXhMCUlMCulKyN9%2B1ltCj%2FrtMdj5ixWcaDOs4t1Y9T4xRbQi8FpMt3Ri91LtHiG71fIJ2a9ShlQ78VgZxreQ9lVaNh2FVoyvFdPvMsDij4l0aBsGZRb7dtSfJiGhYSg6SGbbmv%2FKWStIo%2FqTdeC%2FehaOXUesPJxrloYbLti0jYKFC8MdB0EqipPErA5CuRPOMmKBdy5XInsUlhm0cAL6aKP0lyEhtCsASo3qqZXZQR2xTMWBK%2BvpL2Fc%2FS2r9tIRgzbijezTucOs%2BOdAouuhBVMcKEEsGMfV7EIoOgVA%2BafcThjAhBtN6rLgm24%2FqoY1jOA3iTDPKwp2Zt%2BHk45KNY%2FxTfjTpAmCL%2Fy0tFGWVIsPpKZgbX41Cgc%2Fg8%3D) - - If a new commit is found in the process, **it will keep the lgtm related label, but will automatically remove the `status/can-merge` label** - - When some tests fail, **just trigger the failed test** +- **Phase 1: ** reviewers review code + - when the reviewer performs the `lgtm` action, if the configured sufficient count has not been reached, the bot will add `needs-*-more -lgtm` label. + - If the overall number of reviewer `lgtm` is sufficient, the bot will add `lgtm` and remove `needs-*-more-lgtm` labels. + - Any reviewer doing `/lgtm cancel` will reset the count of `lgtm`. -## Recommended configuration items -### Recommend using Squash mode to merge code +## Recommended configuration items -We recommend using GitHub's Squash mode for merging, because it's a tradition in the TiDB community to create a lot of commits in PR and then automatically Squash them through GitHub when merging. -Our ti-community-merge is currently designed to work in Squash mode, **if you don't use Squash mode, then you are responsible for your own rebase or squash PR in PR, which will disable our ability to store commit hash (see Q&A for details) and eventually cause status/can-merge to be automatically removed due to a new commit**. -So we strongly recommend that you use Squash mode for collaboration. +### It is recommended to use Squash mode to merge code -### It is recommended to turn on the `Require branches to be up to date before merging` branch protection option for small repositories +In terms of merging, we still recommend using GitHub's Squash mode for merging, because this is the current tradition of the TiDB community. Everyone will create a large number of commits in the PR, and then automatically perform Squash through GitHub when merging. At present, our ti-community-merge is also designed to serve the Squash mode. **If you do not use the Squash mode, then you need to be responsible for rebase or squash PR in the PR, which will invalidate our function of storing and submitting the hash (details See Q&A), eventually causing status/can-merge to automatically cancel** because there are new commits. So we strongly recommend that you use Squash mode for collaboration. -Turning this feature on solves the problem we mentioned in the Tide introduction: +### If the CI task of the warehouse is triggered by prow, you need to close the Require branches to be up to date before merging branch protection option. -> - PR1: Rename bifurcate() to bifurcateCrab() -> - PR2: Invoke bifurcate() -> -> In this case, both PRs will be tested with the current master as the base branch, and both PRs will pass. However, once PR1 is merged into the master branch first, and the second PR is merged (because the test also passes), it causes a master error that `bifurcate` is not found. -> +If it is a CI task triggered by prow, the pre-merge with the base has been performed in the checkout link before subsequent construction steps . -This feature requires the PR to merge the current master to the PR before merging, so that our PR uses the latest master branch as the Base test. However, turning this feature on has two effects: -1. PR cannot be merged automatically until the latest master is merged into PR -2. PR merge current master to PR causes `status/can-merge` label to disappear +## Q&A -**The first problem is solved by using [ti-community-tars](en/plugins/tars.md) to automatically update. The second problem is that we can identify the committer of the commit that updates master to PR using the GitHub update button as `web-flow`, so we can determine if we trust that commit based on the committer.** - -## Q&A - -### Why does my own rebase or squash commit cause `status/can-merge` to be removed? - -**Because we are currently storing the hash of the last commit in your PR when tagged with `status/can-merge`**. -When you rebase the PR, the entire hash will change, so it will be untagged automatically. -When you squash the PR yourself, because we store the hash of the last commit and not the hash of the first commit, this will still result in automatic remove label. +### Why does my own rebase or squash commit cause `lgtm` to be removed? +**Because we are currently storing the hash of the last commit in your PR when you tagged `lgtm`**. When you rebase the PR, the entire hash will change, so the tag will be automatically canceled. When you squash PR yourself, because we store the hash of the last submission instead of the hash of the first submission, this will still lead to automatic untagging. diff --git a/docs/plugins/label-blocker.md b/docs/plugins/label-blocker.md index 4ecd5adae..57532ec6d 100644 --- a/docs/plugins/label-blocker.md +++ b/docs/plugins/label-blocker.md @@ -2,7 +2,7 @@ ## 设计背景 -在整个 [PR 协作过程](workflows/pr.md) 中,一些标签会被机器人识别和使用,例如:通过根据 PR 是否带有 `status/can-merge` 标签来判断该 PR 是否能够被合并。对于这些较为敏感的标签,我们不希望它们被别人随意的添加或删除,因此设计了 ti-community-label-blocker 这个插件来帮助我们对这类标签的添加或删除操作进行权限控制。 +在整个 [PR 协作过程](workflows/pr-old.md) 中,一些标签会被机器人识别和使用,例如:通过根据 PR 是否带有 `status/can-merge` 标签来判断该 PR 是否能够被合并。对于这些较为敏感的标签,我们不希望它们被别人随意的添加或删除,因此设计了 ti-community-label-blocker 这个插件来帮助我们对这类标签的添加或删除操作进行权限控制。 ## 权限设计 @@ -23,13 +23,13 @@ ### BlockLabel -| 参数名 | 类型 | 说明 | -| ------------- | -------- | --------------------------------------------------------- | -| regex | string | 匹配标签的正则表达式 | +| 参数名 | 类型 | 说明 | +| ------------- | -------- | --------------------------------------------------------------- | +| regex | string | 匹配标签的正则表达式 | | actions | []string | 匹配的 action 类型, 可填 `labeled` 或 `unlabeled`,至少填写一个 | -| trusted_teams | []string | 信任的 GitHub teams | -| trusted_users | []string | 信任的 GitHub users | -| message | string | 给用户的操作反馈提示,为空表示不提示 | +| trusted_teams | []string | 信任的 GitHub teams | +| trusted_users | []string | 信任的 GitHub users | +| message | string | 给用户的操作反馈提示,为空表示不提示 | 例如: diff --git a/docs/workflows/pr-old.md b/docs/workflows/pr-old.md new file mode 100644 index 000000000..35f6742d0 --- /dev/null +++ b/docs/workflows/pr-old.md @@ -0,0 +1,64 @@ +# PR 工作流 + +## 设计背景 + +在引入 Tide 之后 PR 的协作流程发生了一些变化,但是基本的大框架还是保留了下来。主要的调整发生在 PR 合并阶段,从原来一次性的 `/merge` 命令去触发机器人运行测试合并代码变成了 `/merge` 只负责打上 `status/can-merge` 标签。当 PR 的标签满足要求并且所有测试通过后,PR 将会自动合并无需人为干预。 + +**⚠️ 注意:在阅读以下内容之前请先仔细阅读 [Tide](components/tide.md)、[ti-community-lgtm](plugins/lgtm.md)、[ti-community-tars](plugins/tars.md)、[ti-community-blunderbuss](plugins/blunderbuss.md) 和 [ti-community-merge](plugins/merge.md) 章节内容**。 + +## PR 协作流程 + +- 作者提交 PR +- **第一阶段:** 自动为 PR 请求 reviewers ([ti-community-blunderbuss](plugins/blunderbuss.md) 提供支持) + - 根据当前 PR 所属的 sig 进行自动请求 reviewers + - 根据 ti-community-blunderbuss 配置随机选取多个 reviewers +- **第二阶段:** reviewers review 代码 ([ti-community-lgtm](plugins/lgtm.md) 提供支持) + - reviewer 会查看代码的质量,正确性,工程考量等 + - 如果 reviewer 认为代码没有问题,reviewer 会提交 Approve 意见同意这些改动;如果 reviewer 后续觉得代码还是有问题,可以提交 Request Changes 意见来取消同意代码改动 + - 一旦 reviewer 使用上述命令,机器人 ti-chi-bot 就会自动打上或移除 lgtm 相关标签 +- **第三阶段:** committers 审核代码 ([ti-community-merge](plugins/merge.md) 提供支持) + - committer 对 PR 进行再审核,考察与其他功能的依赖关系,向前/向后的兼容性等 + - 如果 committer 认为代码合并之后不会有问题,committer 会使用 `/merge` 同意这些代码改动;如果 committer 后续觉得代码还是有问题,可以通过 `/merge cancel` 来取消同意这些代码改动合并 + - 一旦 committer 使用上述命令,机器人 ti-chi-bot 就会自动打上或移除 `status/can-merge` 标签 +- **第四阶段:** 自动合并 PR ([Tide](components/tide.md) 提供支持) + - 如果满足以下所有要求 + - 所有要求到的标签已经存在(例如:status/can-merge) + - 无任何的阻止合并的标签(例如:do-not-merge/hold, needs-rebase) + - 如果满足以下任一要求 + - 当前 PR 没有 CI 测试 + - 当前 PR 触发的所有的测试都已经通过 + - 满足上述要求之后,Tide 自动合并 PR + +### 协作流程图 +- [需要两个 lgtm 的 PR 的协作流程图](https://viewer.diagrams.net/?highlight=0000ff&edit=_blank&layers=1&nav=1&title=pr-workflow.drawio#R7Vxdc5s4FP01emwGEB%2FiERw77W62022zm%2B3TDrEVmxaDF%2BM47q9fCYQBIWM5YOO6nslMLCGE4N5zdO%2BRAMDB%2FPUu9hazP6IJDoCmTF4BvAWapuqaSf7Rmk1WY2koq5jG%2FoQ1Kiq%2B%2BD8wq1RY7cqf4GWlYRJFQeIvqpXjKAzxOKnUeXEcravNnqOgetWFN8W1ii9jL6jXPvqTZMZqVdMuDrzH%2FnTGLo00Kzsw9%2FLG7E6WM28SrUtVcAjgII6iJPs1fx3ggD68%2FLk8ftg8Bvffzbvf%2Flz%2B5%2F3l%2Fv7w8e93WWejQ07Z3kKMw6TbrrWs6xcvWLHnxe412eQPMI5W4QTTThQA3VkyD8hPlfz8hpNkwwzurZKIVEVxMoumUegF91G0YO2eozBhzVRaxuHEoYYl5XHgLZf%2B%2BGHmh9mBkR%2Fk3ZMSOwuR0jKJo%2B9bC9LjW3PQYQXeEw5cb%2Fx9mg53EAVRTA6FUYhpVxPiEuyOiiEOi1pysSTe%2FEM7uzHy4lfWd1q4fa2UNqwkaRlmwWW0ise4oR1kAPHiKWb93ePhhxfzk%2Fq8mv47%2FaZ9%2BLieh8zSCr2vkpczu9%2FhaI7JIEmDGAde4r9UoeAxRE237banfop8ch%2BawtC%2FdX2GfdVSql1kA2VnFb5HfpSGUVSlHnmAd8K6dw4RcHXgqGBoATQAzpDWIAUgAwxNgEbAVsBQB64DHB1opjcnXuiGT8tFaizl0%2BcG%2F6ZetZ75Cf6y8FIzrQklVn2%2B7MvkqblT6sDMF%2Fa46GGu8oLjBL82Gjc%2FalaNhFhxXWK73I6zEtHpym53qBjyUKshgdVsaiPbSc03Asi6skyPLJNPzXtpRpekGeaJ75QbrcoX7EJvpaG8SfT8vMQVhtlLVdA8LVXZAqc3gU0YypHw%2Fv2ucwn4yL1frfh%2BAYUz837ZSZZ5oHKjGAidzOF1jvY1Q87hidG9TanZgjZY7o4BIAcszVIax8W3V%2B1Ke%2FIjG0Gn6MttWoJfjF98vCZ1KQ4RcEZgaNDowDFlpqNZNH9aLfcHBBX%2FpxgbeXM%2FoPf%2BHgcvOPHHniBs8AJ%2FGlLkEZfGsRhY5JJ%2BOCUlsyg9pHAmvHzEcMLi7A0F8YQiiCdQB%2FGEEIf1lORQuhQSooA4S3ZtN1VznPE29moipTJ5NSCiRUbQymR1OH5O4Uh8nQLSAs6A%2FtFAPY3eCSAJRF2TYpWE9K6RjjQDcGOguAedO5xf1h7SMfcWNKcIuoVPXK09qFOA5NVPShghpa%2BlIwVCaKFHgOxu1xc%2BzHq02IP5jm0KXdIURp%2Bm0HcF7q6TBgw2cBBTFGiNCVwLuC49hBzgKKnRvISECtro%2Fu6hDsOz5Cto9M1X6DqpcwCQAIrVJ1AM%2BTndodLcAXP6HlRU7X4qjOiiwFeEET7x6m5O72dSP1OQWJIg6TfyVQUrKnL6zz0VT6qmqKWJ1OtJRhk47MDcn0wyq%2BOl%2F8N7SvujxmBZPenccIFx2wQbtvDGTgZbIa1suAb3lFcDmZrfvTbyBhWhycN%2BORHhCFzKiQiG0reIcNVnj6bP5llgnveVzuotC5SdLHakHoWkqyHzWLRVX8PgUcNH39lt1yTdekeIC2UkF0PaasP5gGW1Yd0wOGC304bF05Qgw7uGVVy4JBNXwZZxldzCnaXI%2BWpn7lHXKtsIAHXfOksBwNT6FgDUX0MDU2W39Ki9qmBqfavNIJrP%2FSRh6b1Bc3i67cakPu%2FW5ZuzzOFNu3c3FwSel%2BjmshKWivp0c01E9heanduNSCHJOYuLzjcfV%2BuSo4CIrpl5m8zcEq1cijJzfutndwxZl12uqfkFp%2BbyU8WOhKOn5JzLqVF%2BHwcn5zaHQHik5BxyA1aMxnHx7S21241b4glZIJdfYniEJH2%2B9Ub%2BdlQs2rr9xlx47IXv5pjcbn3yPceEGMnu4OHR2l1savbh%2BYUsddjMcmzEbN8V24sYvU%2FEQAF%2FHRi8%2FHSZReaoTet%2B%2BhEn5q5mnhzHVxX4ILjB06jAqsIrMkeWgWG%2BUfrylQG4Y4NEoQxUTJGvpbW0sFo1cL4F8vhA10SWNel%2BJORSyzojYBtXFaGVimDLviRwNBVB62VD4dFpWXYL7ZFeANa5XBXx%2BzM6ylV13p%2FM5ly11r6aq%2B6dXuyT5LaircXNry8jYJtpNnVLX2WmidYtjSWLNjblIXKU0hXhMCUlMCulKyN9%2B1ltCj%2FrtMdj5ixWcaDOs4t1Y9T4xRbQi8FpMt3Ri91LtHiG71fIJ2a9ShlQ78VgZxreQ9lVaNh2FVoyvFdPvMsDij4l0aBsGZRb7dtSfJiGhYSg6SGbbmv%2FKWStIo%2FqTdeC%2FehaOXUesPJxrloYbLti0jYKFC8MdB0EqipPErA5CuRPOMmKBdy5XInsUlhm0cAL6aKP0lyEhtCsASo3qqZXZQR2xTMWBK%2BvpL2Fc%2FS2r9tIRgzbijezTucOs%2BOdAouuhBVMcKEEsGMfV7EIoOgVA%2BafcThjAhBtN6rLgm24%2FqoY1jOA3iTDPKwp2Zt%2BHk45KNY%2FxTfjTpAmCL%2Fy0tFGWVIsPpKZgbX41Cgc%2Fg8%3D) + - 流程中如果发现新的提交,**会保持 lgtm 相关 label,但是会自动取消 `status/can-merge` 标签** + - 在某些测试不通过时,**只需触发失败的测试即可** + + +## 推荐配置项 + +### 推荐使用 Squash 模式合并代码 + +在合并方式上我们还是推荐采用 GitHub 的 Squash 模式进行合并,因为这是目前 TiDB 社区的传统,大家都会在 PR 中创建大量提交,然后在合并时通过 GitHub 自动进行 Squash。目前我们的 ti-community-merge 的设计也是为 Squash 模式服务,**如果不采用 Squash 模式,那么你在 PR 中就需要自己负责 rebase 或者 squash PR,这样会使我们存储提交 hash 的功能失效(详见 Q&A),最终导致 status/can-merge 因为有新的提交而自动取消**。所以我们强烈建议大家使用 Squash 模式进行协作。 + +### 小型仓库推荐打开 Require branches to be up to date before merging 分支保护选项 + +打开这个功能就能解决我们在 Tide 介绍中提到的问题: + +> - PR1: 重命名 bifurcate() 为 bifurcateCrab() +> - PR2: 调用 bifurcate() +> +> 这个时候两个 PR 都会以当前 master 作为 Base 分支进行测试,两个 PR 都会通过。但是一旦 PR1 先合并入 master 分支,第二个 PR 合并之后(因为测试也通过了),就会导致 master 出现找不到 `bifurcate` 的错误。 +> + +该功能要求 PR 在合并之前必须合并当前 master 到 PR,这样就保证我们的 PR 用了最新的 master 分支作为 Base 测试。但是打开这个功能之后有两个影响: +1. PR 在合并最新的 master 到 PR 之前无法自动合并 +2. PR 合并当前 master 到 PR 导致 `status/can-merge` 标签消失 + +**第一个问题使用 [ti-community-tars](plugins/tars.md) 自动更新就可以解决。第二个问题因为我们可以识别到使用 GitHub 更新按钮更新 master 到 PR 的提交的 committer 为 `web-flow`,所以可以根据 committer 来判断是否信任该提交。** + +## Q&A + +### 为什么我自己 rebase 或者 squash 提交会导致 `status/can-merge` 被移除? + +**因为我们目前存储的是打上 `status/can-merge` 标签时你 PR 中的最后一个提交的 hash**。当你 rebase PR 之后整个 hash 都会发生变化,所以会自动取消标签。当你自己 squash PR 的时候,因为我们存储的是最后一个提交的 hash 不是第一个提交的 hash,这样还是会导致自动取消标签。 diff --git a/docs/workflows/pr.md b/docs/workflows/pr.md index 35f6742d0..f077a3d1f 100644 --- a/docs/workflows/pr.md +++ b/docs/workflows/pr.md @@ -2,37 +2,18 @@ ## 设计背景 -在引入 Tide 之后 PR 的协作流程发生了一些变化,但是基本的大框架还是保留了下来。主要的调整发生在 PR 合并阶段,从原来一次性的 `/merge` 命令去触发机器人运行测试合并代码变成了 `/merge` 只负责打上 `status/can-merge` 标签。当 PR 的标签满足要求并且所有测试通过后,PR 将会自动合并无需人为干预。 - -**⚠️ 注意:在阅读以下内容之前请先仔细阅读 [Tide](components/tide.md)、[ti-community-lgtm](plugins/lgtm.md)、[ti-community-tars](plugins/tars.md)、[ti-community-blunderbuss](plugins/blunderbuss.md) 和 [ti-community-merge](plugins/merge.md) 章节内容**。 +我们使用 prow 原生的 lgtm + approve 相关的插件来进行流程驱动,但我们在此基础上增加了可选配置多人 lgtm 的功能。 ## PR 协作流程 -- 作者提交 PR -- **第一阶段:** 自动为 PR 请求 reviewers ([ti-community-blunderbuss](plugins/blunderbuss.md) 提供支持) - - 根据当前 PR 所属的 sig 进行自动请求 reviewers - - 根据 ti-community-blunderbuss 配置随机选取多个 reviewers -- **第二阶段:** reviewers review 代码 ([ti-community-lgtm](plugins/lgtm.md) 提供支持) - - reviewer 会查看代码的质量,正确性,工程考量等 - - 如果 reviewer 认为代码没有问题,reviewer 会提交 Approve 意见同意这些改动;如果 reviewer 后续觉得代码还是有问题,可以提交 Request Changes 意见来取消同意代码改动 - - 一旦 reviewer 使用上述命令,机器人 ti-chi-bot 就会自动打上或移除 lgtm 相关标签 -- **第三阶段:** committers 审核代码 ([ti-community-merge](plugins/merge.md) 提供支持) - - committer 对 PR 进行再审核,考察与其他功能的依赖关系,向前/向后的兼容性等 - - 如果 committer 认为代码合并之后不会有问题,committer 会使用 `/merge` 同意这些代码改动;如果 committer 后续觉得代码还是有问题,可以通过 `/merge cancel` 来取消同意这些代码改动合并 - - 一旦 committer 使用上述命令,机器人 ti-chi-bot 就会自动打上或移除 `status/can-merge` 标签 -- **第四阶段:** 自动合并 PR ([Tide](components/tide.md) 提供支持) - - 如果满足以下所有要求 - - 所有要求到的标签已经存在(例如:status/can-merge) - - 无任何的阻止合并的标签(例如:do-not-merge/hold, needs-rebase) - - 如果满足以下任一要求 - - 当前 PR 没有 CI 测试 - - 当前 PR 触发的所有的测试都已经通过 - - 满足上述要求之后,Tide 自动合并 PR +大部分内容与上游 prow 原生的流程相同,建议先阅读[这里](https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#code-review-using-owners-files)。 + +以下是相对上游原生差异的部分: -### 协作流程图 -- [需要两个 lgtm 的 PR 的协作流程图](https://viewer.diagrams.net/?highlight=0000ff&edit=_blank&layers=1&nav=1&title=pr-workflow.drawio#R7Vxdc5s4FP01emwGEB%2FiERw77W62022zm%2B3TDrEVmxaDF%2BM47q9fCYQBIWM5YOO6nslMLCGE4N5zdO%2BRAMDB%2FPUu9hazP6IJDoCmTF4BvAWapuqaSf7Rmk1WY2koq5jG%2FoQ1Kiq%2B%2BD8wq1RY7cqf4GWlYRJFQeIvqpXjKAzxOKnUeXEcravNnqOgetWFN8W1ii9jL6jXPvqTZMZqVdMuDrzH%2FnTGLo00Kzsw9%2FLG7E6WM28SrUtVcAjgII6iJPs1fx3ggD68%2FLk8ftg8Bvffzbvf%2Flz%2B5%2F3l%2Fv7w8e93WWejQ07Z3kKMw6TbrrWs6xcvWLHnxe412eQPMI5W4QTTThQA3VkyD8hPlfz8hpNkwwzurZKIVEVxMoumUegF91G0YO2eozBhzVRaxuHEoYYl5XHgLZf%2B%2BGHmh9mBkR%2Fk3ZMSOwuR0jKJo%2B9bC9LjW3PQYQXeEw5cb%2Fx9mg53EAVRTA6FUYhpVxPiEuyOiiEOi1pysSTe%2FEM7uzHy4lfWd1q4fa2UNqwkaRlmwWW0ise4oR1kAPHiKWb93ePhhxfzk%2Fq8mv47%2FaZ9%2BLieh8zSCr2vkpczu9%2FhaI7JIEmDGAde4r9UoeAxRE237banfop8ch%2BawtC%2FdX2GfdVSql1kA2VnFb5HfpSGUVSlHnmAd8K6dw4RcHXgqGBoATQAzpDWIAUgAwxNgEbAVsBQB64DHB1opjcnXuiGT8tFaizl0%2BcG%2F6ZetZ75Cf6y8FIzrQklVn2%2B7MvkqblT6sDMF%2Fa46GGu8oLjBL82Gjc%2FalaNhFhxXWK73I6zEtHpym53qBjyUKshgdVsaiPbSc03Asi6skyPLJNPzXtpRpekGeaJ75QbrcoX7EJvpaG8SfT8vMQVhtlLVdA8LVXZAqc3gU0YypHw%2Fv2ucwn4yL1frfh%2BAYUz837ZSZZ5oHKjGAidzOF1jvY1Q87hidG9TanZgjZY7o4BIAcszVIax8W3V%2B1Ke%2FIjG0Gn6MttWoJfjF98vCZ1KQ4RcEZgaNDowDFlpqNZNH9aLfcHBBX%2FpxgbeXM%2FoPf%2BHgcvOPHHniBs8AJ%2FGlLkEZfGsRhY5JJ%2BOCUlsyg9pHAmvHzEcMLi7A0F8YQiiCdQB%2FGEEIf1lORQuhQSooA4S3ZtN1VznPE29moipTJ5NSCiRUbQymR1OH5O4Uh8nQLSAs6A%2FtFAPY3eCSAJRF2TYpWE9K6RjjQDcGOguAedO5xf1h7SMfcWNKcIuoVPXK09qFOA5NVPShghpa%2BlIwVCaKFHgOxu1xc%2BzHq02IP5jm0KXdIURp%2Bm0HcF7q6TBgw2cBBTFGiNCVwLuC49hBzgKKnRvISECtro%2Fu6hDsOz5Cto9M1X6DqpcwCQAIrVJ1AM%2BTndodLcAXP6HlRU7X4qjOiiwFeEET7x6m5O72dSP1OQWJIg6TfyVQUrKnL6zz0VT6qmqKWJ1OtJRhk47MDcn0wyq%2BOl%2F8N7SvujxmBZPenccIFx2wQbtvDGTgZbIa1suAb3lFcDmZrfvTbyBhWhycN%2BORHhCFzKiQiG0reIcNVnj6bP5llgnveVzuotC5SdLHakHoWkqyHzWLRVX8PgUcNH39lt1yTdekeIC2UkF0PaasP5gGW1Yd0wOGC304bF05Qgw7uGVVy4JBNXwZZxldzCnaXI%2BWpn7lHXKtsIAHXfOksBwNT6FgDUX0MDU2W39Ki9qmBqfavNIJrP%2FSRh6b1Bc3i67cakPu%2FW5ZuzzOFNu3c3FwSel%2BjmshKWivp0c01E9heanduNSCHJOYuLzjcfV%2BuSo4CIrpl5m8zcEq1cijJzfutndwxZl12uqfkFp%2BbyU8WOhKOn5JzLqVF%2BHwcn5zaHQHik5BxyA1aMxnHx7S21241b4glZIJdfYniEJH2%2B9Ub%2BdlQs2rr9xlx47IXv5pjcbn3yPceEGMnu4OHR2l1savbh%2BYUsddjMcmzEbN8V24sYvU%2FEQAF%2FHRi8%2FHSZReaoTet%2B%2BhEn5q5mnhzHVxX4ILjB06jAqsIrMkeWgWG%2BUfrylQG4Y4NEoQxUTJGvpbW0sFo1cL4F8vhA10SWNel%2BJORSyzojYBtXFaGVimDLviRwNBVB62VD4dFpWXYL7ZFeANa5XBXx%2BzM6ylV13p%2FM5ly11r6aq%2B6dXuyT5LaircXNry8jYJtpNnVLX2WmidYtjSWLNjblIXKU0hXhMCUlMCulKyN9%2B1ltCj%2FrtMdj5ixWcaDOs4t1Y9T4xRbQi8FpMt3Ri91LtHiG71fIJ2a9ShlQ78VgZxreQ9lVaNh2FVoyvFdPvMsDij4l0aBsGZRb7dtSfJiGhYSg6SGbbmv%2FKWStIo%2FqTdeC%2FehaOXUesPJxrloYbLti0jYKFC8MdB0EqipPErA5CuRPOMmKBdy5XInsUlhm0cAL6aKP0lyEhtCsASo3qqZXZQR2xTMWBK%2BvpL2Fc%2FS2r9tIRgzbijezTucOs%2BOdAouuhBVMcKEEsGMfV7EIoOgVA%2BafcThjAhBtN6rLgm24%2FqoY1jOA3iTDPKwp2Zt%2BHk45KNY%2FxTfjTpAmCL%2Fy0tFGWVIsPpKZgbX41Cgc%2Fg8%3D) - - 流程中如果发现新的提交,**会保持 lgtm 相关 label,但是会自动取消 `status/can-merge` 标签** - - 在某些测试不通过时,**只需触发失败的测试即可** +- **Phase 1:** reviewers review 代码 + - 当 reviewer 进行 `lgtm` 动作时, 如果还没有达到配置的足够数量计数, bot 将会添加 `needs-*-more-lgtm` 标签。 + - 如果 reviewer `lgtm` 后整体的数量足够, bot 将会添加 `lgtm` 并移除 `needs-*-more-lgtm` 标签。 + - 任何 reviewer 进行 `/lgtm cancel` 则会重置 `lgtm` 的计数。 ## 推荐配置项 @@ -41,24 +22,12 @@ 在合并方式上我们还是推荐采用 GitHub 的 Squash 模式进行合并,因为这是目前 TiDB 社区的传统,大家都会在 PR 中创建大量提交,然后在合并时通过 GitHub 自动进行 Squash。目前我们的 ti-community-merge 的设计也是为 Squash 模式服务,**如果不采用 Squash 模式,那么你在 PR 中就需要自己负责 rebase 或者 squash PR,这样会使我们存储提交 hash 的功能失效(详见 Q&A),最终导致 status/can-merge 因为有新的提交而自动取消**。所以我们强烈建议大家使用 Squash 模式进行协作。 -### 小型仓库推荐打开 Require branches to be up to date before merging 分支保护选项 - -打开这个功能就能解决我们在 Tide 介绍中提到的问题: - -> - PR1: 重命名 bifurcate() 为 bifurcateCrab() -> - PR2: 调用 bifurcate() -> -> 这个时候两个 PR 都会以当前 master 作为 Base 分支进行测试,两个 PR 都会通过。但是一旦 PR1 先合并入 master 分支,第二个 PR 合并之后(因为测试也通过了),就会导致 master 出现找不到 `bifurcate` 的错误。 -> - -该功能要求 PR 在合并之前必须合并当前 master 到 PR,这样就保证我们的 PR 用了最新的 master 分支作为 Base 测试。但是打开这个功能之后有两个影响: -1. PR 在合并最新的 master 到 PR 之前无法自动合并 -2. PR 合并当前 master 到 PR 导致 `status/can-merge` 标签消失 +### 如果仓库的 CI 任务是 prow 触发的需关闭 Require branches to be up to date before merging 分支保护选项 -**第一个问题使用 [ti-community-tars](plugins/tars.md) 自动更新就可以解决。第二个问题因为我们可以识别到使用 GitHub 更新按钮更新 master 到 PR 的提交的 committer 为 `web-flow`,所以可以根据 committer 来判断是否信任该提交。** +如果是 prow 触发的 CI 任务, 在 checkout 环节已经进行了与 base 的预合并再进行后续构建步骤。 ## Q&A -### 为什么我自己 rebase 或者 squash 提交会导致 `status/can-merge` 被移除? +### 为什么我自己 rebase 或者 squash 提交会导致 `lgtm` 被移除? -**因为我们目前存储的是打上 `status/can-merge` 标签时你 PR 中的最后一个提交的 hash**。当你 rebase PR 之后整个 hash 都会发生变化,所以会自动取消标签。当你自己 squash PR 的时候,因为我们存储的是最后一个提交的 hash 不是第一个提交的 hash,这样还是会导致自动取消标签。 +**因为我们目前存储的是打上 `lgtm` 标签时你 PR 中的最后一个提交的 hash**。当你 rebase PR 之后整个 hash 都会发生变化,所以会自动取消标签。当你自己 squash PR 的时候,因为我们存储的是最后一个提交的 hash 不是第一个提交的 hash,这样还是会导致自动取消标签。 From 6c932ad9a747a1f8c3de83d7a31516718008322d Mon Sep 17 00:00:00 2001 From: wuhuizuo Date: Wed, 7 Jun 2023 16:15:17 +0800 Subject: [PATCH 2/9] docs: update new workflow documents en edition Signed-off-by: wuhuizuo --- docs/en/workflows/pr.md | 55 ++++++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/docs/en/workflows/pr.md b/docs/en/workflows/pr.md index 0ace49f89..9c92eef09 100644 --- a/docs/en/workflows/pr.md +++ b/docs/en/workflows/pr.md @@ -4,17 +4,48 @@ We use prow's native lgtm + approve related plug-ins to drive the process, but we have added the optional configuration of multi-person lgtm on this basis. -## Most of the PR collaboration process - -It's the same as the upstream prow native process, it is recommended to read [here](https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#code-review -using-owners-files). - -The following is the part of the difference relative to the original upstream: - -- **Phase 1: ** reviewers review code - - when the reviewer performs the `lgtm` action, if the configured sufficient count has not been reached, the bot will add `needs-*-more -lgtm` label. - - If the overall number of reviewer `lgtm` is sufficient, the bot will add `lgtm` and remove `needs-*-more-lgtm` labels. - - Any reviewer doing `/lgtm cancel` will reset the count of `lgtm`. +## Main PR Collaboration Process +- The **author** submits a PR +- Phase 0: Automation suggests **[reviewers][reviewer-role]** and **[approvers][approver-role]** for the PR + - Determine the set of OWNERS files nearest to the code being changed + - Choose at least two suggested **reviewers**, trying to find a unique reviewer for every leaf + OWNERS file, and request their reviews on the PR + - Choose suggested **approvers**, one from each OWNERS file, and list them in a comment on the PR +- Phase 1: Humans review the PR + - **Reviewers** look for general code quality, correctness, sane software engineering, style, etc. + - Anyone in the organization can act as a **reviewer** with the exception of the individual who + opened the PR + - If the code changes look good to them, a **reviewer** types `/lgtm` in a PR comment or review; + if they change their mind, they `/lgtm cancel`; + if the configured sufficient lgtm count has not been reached, the bot will add `needs-*-more -lgtm` label. + - If sufficient **reviewers** have `/lgtm`'ed, [prow](https://prow.tidb.net) + ([@ti-chi-bot](https://github.com/apps/ti-chi-bot)) applies an `lgtm` label and remove `needs-*-more-lgtm` label to the PR; + - Any reviewer doing `/lgtm cancel` will reset the count of `lgtm`. +- Phase 2: Humans approve the PR + - The PR **author** `/assign`'s all suggested **approvers** to the PR, and optionally notifies + them (eg: "pinging @foo for approval") + - Only people listed in the relevant OWNERS files, either directly or through an alias, as [described + above](#owners_aliases), can act as **approvers**, including the individual who opened the PR. + - **Approvers** look for holistic acceptance criteria, including dependencies with other features, + forwards/backwards compatibility, API and flag definitions, etc + - If the code changes look good to them, an **approver** types `/approve` in a PR comment or + review; if they change their mind, they `/approve cancel` + - [prow](https://prow.tidb.net) ([@ti-chi-bot](https://github.com/apps/ti-chi-bot)) updates its + comment in the PR to indicate which **approvers** still need to approve + - Once all **approvers** (one from each of the previously identified OWNERS files) have approved, + [prow](https://prow.tidb.net) ([@ti-chi-bot](https://github.com/apps/ti-chi-bot)) applies an + `approved` label +- Phase 3: Automation merges the PR: + - If all of the following are true: + - All required labels are present (eg: `lgtm`, `approved`) + - Any blocking labels are missing (eg: there is no `do-not-merge/hold`, `needs-rebase`) + - And if any of the following are true: + - there are no presubmit prow jobs configured for this repo + - there are presubmit prow jobs configured for this repo, and they all pass after automatically + being re-run one last time + - Then the PR will automatically be merged +> It's hacked base on the [kubernetes community review process](https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#code-review-using-owners-files). ## Recommended configuration items @@ -22,7 +53,7 @@ The following is the part of the difference relative to the original upstream: In terms of merging, we still recommend using GitHub's Squash mode for merging, because this is the current tradition of the TiDB community. Everyone will create a large number of commits in the PR, and then automatically perform Squash through GitHub when merging. At present, our ti-community-merge is also designed to serve the Squash mode. **If you do not use the Squash mode, then you need to be responsible for rebase or squash PR in the PR, which will invalidate our function of storing and submitting the hash (details See Q&A), eventually causing status/can-merge to automatically cancel** because there are new commits. So we strongly recommend that you use Squash mode for collaboration. -### If the CI task of the warehouse is triggered by prow, you need to close the Require branches to be up to date before merging branch protection option. +### If the CI task of the repository is triggered by prow, you need to close the Require branches to be up to date before merging branch protection option. If it is a CI task triggered by prow, the pre-merge with the base has been performed in the checkout link before subsequent construction steps . @@ -30,4 +61,4 @@ If it is a CI task triggered by prow, the pre-merge with the base has been perfo ### Why does my own rebase or squash commit cause `lgtm` to be removed? -**Because we are currently storing the hash of the last commit in your PR when you tagged `lgtm`**. When you rebase the PR, the entire hash will change, so the tag will be automatically canceled. When you squash PR yourself, because we store the hash of the last submission instead of the hash of the first submission, this will still lead to automatic untagging. +**Because we are currently storing the hash of the last commit in your PR when you tagged `lgtm`**. When you rebase the PR, the entire hash will change, so the label will be automatically removed. When you squash PR yourself, because we store the hash of the last submission instead of the hash of the first submission, this will still lead to automatic removing. From 445ed86c2bfe7e30c6bb6edc335f9460a7e00774 Mon Sep 17 00:00:00 2001 From: wuhuizuo Date: Wed, 7 Jun 2023 16:41:07 +0800 Subject: [PATCH 3/9] docs: update new workflow documents Signed-off-by: wuhuizuo --- docs/en/workflows/pr.md | 4 ++-- docs/workflows/pr.md | 31 ++++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/docs/en/workflows/pr.md b/docs/en/workflows/pr.md index 9c92eef09..783799c22 100644 --- a/docs/en/workflows/pr.md +++ b/docs/en/workflows/pr.md @@ -20,7 +20,7 @@ We use prow's native lgtm + approve related plug-ins to drive the process, but w if the configured sufficient lgtm count has not been reached, the bot will add `needs-*-more -lgtm` label. - If sufficient **reviewers** have `/lgtm`'ed, [prow](https://prow.tidb.net) ([@ti-chi-bot](https://github.com/apps/ti-chi-bot)) applies an `lgtm` label and remove `needs-*-more-lgtm` label to the PR; - - Any reviewer doing `/lgtm cancel` will reset the count of `lgtm`. + - Any valid reviewer or the PR author doing `/lgtm cancel` will reset the count of `lgtm`. - Phase 2: Humans approve the PR - The PR **author** `/assign`'s all suggested **approvers** to the PR, and optionally notifies them (eg: "pinging @foo for approval") @@ -45,7 +45,7 @@ We use prow's native lgtm + approve related plug-ins to drive the process, but w being re-run one last time - Then the PR will automatically be merged -> It's hacked base on the [kubernetes community review process](https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#code-review-using-owners-files). +> Modified based on [kubernetes community review process](https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#code-review-using-owners-files). ## Recommended configuration items diff --git a/docs/workflows/pr.md b/docs/workflows/pr.md index f077a3d1f..f56eb5167 100644 --- a/docs/workflows/pr.md +++ b/docs/workflows/pr.md @@ -6,7 +6,7 @@ ## PR 协作流程 -大部分内容与上游 prow 原生的流程相同,建议先阅读[这里](https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#code-review-using-owners-files)。 + 以下是相对上游原生差异的部分: @@ -15,6 +15,35 @@ - 如果 reviewer `lgtm` 后整体的数量足够, bot 将会添加 `lgtm` 并移除 `needs-*-more-lgtm` 标签。 - 任何 reviewer 进行 `/lgtm cancel` 则会重置 `lgtm` 的计数。 +- 作者提交 PR 。 +- 阶段 0:自动化建议 PR 的 [reviewers][reviewer-role] 和 [approvers][approver-role] + - 确定最接近被更改代码的 OWNERS 文件集。 + - 至少选择两个建议的审阅者,尝试为每个叶子 OWNERS 文件找到一个独特的审阅者,并请求他们对 PR 进行审阅。 + - 从每个 OWNERS 文件中选择建议的批准人,并将他们列在 PR 的评论中。 +- 阶段 1:人工审查 PR + - 审阅者寻找一般的代码质量、正确性、健全的软件工程、风格等。 + - 组织中的任何人都可以担任审阅者,但打开 PR 作者除外。 + - 如果代码更改对他们来说很好,审阅者会在 PR 中 或者在 review 中评论 `/lgtm`;如果他们改变主意,他们可以评论 `/lgtm cancel`;- 如果未达到配置的足够 lgtm 计数,机器人将添加 `needs-*-more-lgtm` 标签。 + - 如果有足够多的审阅者已经 `/lgtm` [prow](https://prow.tidb.net) ([@ti-chi-bot](https://github.com/apps/ti-chi-bot)) 会为 PR 添加 `lgtm` 标签并删除 `needs-*-more-lgtm` 标签; + - 任何有效的 reviewer 或者 PR 作者进行 `/lgtm cancel` 都会重置 lgtm 计数。 +- 阶段 2:人工批准 PR + - PR作者可向 PR 评论 `/assign`,这将推荐批准者,也可选择通知他们(例如:“pinging @foo for approval”)。 + - 只有相关 OWNERS 文件中列出的人,无论是直接还是通过别名,如上所述,都可以充当批准人,包括打开 PR 的个人。 + - 审批者寻找整体验收标准,包括与其他功能的依赖性、向前/向后兼容性、API 和标志定义等。 + - 如果代码更改对他们来说很好,批准者会/approve输入 PR 评论或评论;如果他们改变主意,他们可以 `/approve cancel` + - [prow](https://prow.tidb.net) ([@ti-chi-bot](https://github.com/apps/ti-chi-bot)) 更新了它在 PR 中的评论以表明哪些批准者仍然需要批准。 + 一 旦所有批准者(来自每个先前确定的 OWNERS 文件的批准者)都已批准,[prow](https://prow.tidb.net) ([@ti-chi-bot](https://github.com/apps/ti-chi-bot)) 应用 `approved` 标签。 +- 阶段 3:自动化合并 PR: + - 如果以下所有情况都为真: + - 所有必需的标签都存在(例如:lgtm,approved) + - 缺少任何阻塞标签(例如:没有do-not-merge/hold, needs-rebase) + - 如果以下任何一项为真: + - 没有为此 repo 配置的预提交 prow 作业 + - 有为此 repo 配置的预提交 prow 作业,它们在最后一次自动重新运行后全部通过 + - 然后 PR 会自动合并 + +> 基于 [kubernetes 社区 PR 评审流程](https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#code-review-using-owners-files)修改调整而来。 + ## 推荐配置项 From 5d9b84363e1400d320fc6a5189e9f7b1c5dae9e3 Mon Sep 17 00:00:00 2001 From: wuhuizuo Date: Thu, 8 Jun 2023 23:58:28 +0800 Subject: [PATCH 4/9] Update docs/_sidebar.md Co-authored-by: Mini256 --- docs/_sidebar.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/_sidebar.md b/docs/_sidebar.md index 7d3644cf3..5e902512d 100644 --- a/docs/_sidebar.md +++ b/docs/_sidebar.md @@ -29,5 +29,5 @@ - [autobump](tools/autobump.md) - [工作流](workflows.md) - [PR 工作流](workflows/pr.md) - - [PR 工作流(老)](workflows/pr-old.md) + - [PR 工作流(旧)](workflows/pr-old.md) From a55cf139ae1fcfd46972b2379c139c5c21a245f5 Mon Sep 17 00:00:00 2001 From: wuhuizuo Date: Thu, 8 Jun 2023 23:59:39 +0800 Subject: [PATCH 5/9] Update docs/en/workflows/pr.md Co-authored-by: Mini256 --- docs/en/workflows/pr.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/workflows/pr.md b/docs/en/workflows/pr.md index 783799c22..1f5ea336c 100644 --- a/docs/en/workflows/pr.md +++ b/docs/en/workflows/pr.md @@ -51,7 +51,7 @@ We use prow's native lgtm + approve related plug-ins to drive the process, but w ### It is recommended to use Squash mode to merge code -In terms of merging, we still recommend using GitHub's Squash mode for merging, because this is the current tradition of the TiDB community. Everyone will create a large number of commits in the PR, and then automatically perform Squash through GitHub when merging. At present, our ti-community-merge is also designed to serve the Squash mode. **If you do not use the Squash mode, then you need to be responsible for rebase or squash PR in the PR, which will invalidate our function of storing and submitting the hash (details See Q&A), eventually causing status/can-merge to automatically cancel** because there are new commits. So we strongly recommend that you use Squash mode for collaboration. +In terms of merging, we still recommend using GitHub's Squash mode for merging, because this is the current tradition of the TiDB community. Everyone will create a large number of commits in the PR, and then automatically perform Squash through GitHub when merging. At present, our ti-community-merge is also designed to serve the Squash mode. **If you do not use the Squash mode, then you need to be responsible for rebase or squash commits in the PR, which will invalidate our function of storing and submitting the hash (details See Q&A), eventually causing status/can-merge to automatically cancel** because there are new commits. So we strongly recommend that you use Squash mode for collaboration. ### If the CI task of the repository is triggered by prow, you need to close the Require branches to be up to date before merging branch protection option. From edaf4a251366ef69cf8949d3822e57c8917bcbe9 Mon Sep 17 00:00:00 2001 From: wuhuizuo Date: Thu, 8 Jun 2023 23:59:55 +0800 Subject: [PATCH 6/9] Update docs/en/workflows/pr.md Co-authored-by: Mini256 --- docs/en/workflows/pr.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/workflows/pr.md b/docs/en/workflows/pr.md index 1f5ea336c..b16b68948 100644 --- a/docs/en/workflows/pr.md +++ b/docs/en/workflows/pr.md @@ -53,7 +53,7 @@ We use prow's native lgtm + approve related plug-ins to drive the process, but w In terms of merging, we still recommend using GitHub's Squash mode for merging, because this is the current tradition of the TiDB community. Everyone will create a large number of commits in the PR, and then automatically perform Squash through GitHub when merging. At present, our ti-community-merge is also designed to serve the Squash mode. **If you do not use the Squash mode, then you need to be responsible for rebase or squash commits in the PR, which will invalidate our function of storing and submitting the hash (details See Q&A), eventually causing status/can-merge to automatically cancel** because there are new commits. So we strongly recommend that you use Squash mode for collaboration. -### If the CI task of the repository is triggered by prow, you need to close the Require branches to be up to date before merging branch protection option. +### If the CI task of the repository is triggered by prow, you need to trun off the "Require branches to be up to date before merging branch protection" option. If it is a CI task triggered by prow, the pre-merge with the base has been performed in the checkout link before subsequent construction steps . From 68d469ec00f4ab4ede80a420a37bf98192226b23 Mon Sep 17 00:00:00 2001 From: wuhuizuo Date: Fri, 9 Jun 2023 00:53:25 +0800 Subject: [PATCH 7/9] Update pr.md --- docs/workflows/pr.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docs/workflows/pr.md b/docs/workflows/pr.md index f56eb5167..0847288ae 100644 --- a/docs/workflows/pr.md +++ b/docs/workflows/pr.md @@ -6,10 +6,6 @@ ## PR 协作流程 - - -以下是相对上游原生差异的部分: - - **Phase 1:** reviewers review 代码 - 当 reviewer 进行 `lgtm` 动作时, 如果还没有达到配置的足够数量计数, bot 将会添加 `needs-*-more-lgtm` 标签。 - 如果 reviewer `lgtm` 后整体的数量足够, bot 将会添加 `lgtm` 并移除 `needs-*-more-lgtm` 标签。 From a29f2a6ff808ac1da94d3ece6841302ea3a451a6 Mon Sep 17 00:00:00 2001 From: wuhuizuo Date: Fri, 9 Jun 2023 00:55:46 +0800 Subject: [PATCH 8/9] Update pr.md --- docs/workflows/pr.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/docs/workflows/pr.md b/docs/workflows/pr.md index 0847288ae..8efbf142b 100644 --- a/docs/workflows/pr.md +++ b/docs/workflows/pr.md @@ -6,11 +6,6 @@ ## PR 协作流程 -- **Phase 1:** reviewers review 代码 - - 当 reviewer 进行 `lgtm` 动作时, 如果还没有达到配置的足够数量计数, bot 将会添加 `needs-*-more-lgtm` 标签。 - - 如果 reviewer `lgtm` 后整体的数量足够, bot 将会添加 `lgtm` 并移除 `needs-*-more-lgtm` 标签。 - - 任何 reviewer 进行 `/lgtm cancel` 则会重置 `lgtm` 的计数。 - - 作者提交 PR 。 - 阶段 0:自动化建议 PR 的 [reviewers][reviewer-role] 和 [approvers][approver-role] - 确定最接近被更改代码的 OWNERS 文件集。 From dce5f0e64bcaa2cbb707329dab6bbb4a53dad96b Mon Sep 17 00:00:00 2001 From: wuhuizuo Date: Fri, 9 Jun 2023 12:01:35 +0800 Subject: [PATCH 9/9] docs: add OWNERS spec links Signed-off-by: wuhuizuo --- docs/en/workflows/pr.md | 2 +- docs/workflows/pr.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/en/workflows/pr.md b/docs/en/workflows/pr.md index b16b68948..ddef329c8 100644 --- a/docs/en/workflows/pr.md +++ b/docs/en/workflows/pr.md @@ -7,7 +7,7 @@ We use prow's native lgtm + approve related plug-ins to drive the process, but w ## Main PR Collaboration Process - The **author** submits a PR - Phase 0: Automation suggests **[reviewers][reviewer-role]** and **[approvers][approver-role]** for the PR - - Determine the set of OWNERS files nearest to the code being changed + - Determine the set of OWNERS([spec](https://www.kubernetes.dev/docs/guide/owners/#owners-spec)) files nearest to the code being changed - Choose at least two suggested **reviewers**, trying to find a unique reviewer for every leaf OWNERS file, and request their reviews on the PR - Choose suggested **approvers**, one from each OWNERS file, and list them in a comment on the PR diff --git a/docs/workflows/pr.md b/docs/workflows/pr.md index 8efbf142b..8e1a7028a 100644 --- a/docs/workflows/pr.md +++ b/docs/workflows/pr.md @@ -8,7 +8,7 @@ - 作者提交 PR 。 - 阶段 0:自动化建议 PR 的 [reviewers][reviewer-role] 和 [approvers][approver-role] - - 确定最接近被更改代码的 OWNERS 文件集。 + - 确定最接近被更改代码的 OWNERS([spec](https://www.kubernetes.dev/docs/guide/owners/#owners-spec)) 文件集。 - 至少选择两个建议的审阅者,尝试为每个叶子 OWNERS 文件找到一个独特的审阅者,并请求他们对 PR 进行审阅。 - 从每个 OWNERS 文件中选择建议的批准人,并将他们列在 PR 的评论中。 - 阶段 1:人工审查 PR @@ -18,7 +18,7 @@ - 如果有足够多的审阅者已经 `/lgtm` [prow](https://prow.tidb.net) ([@ti-chi-bot](https://github.com/apps/ti-chi-bot)) 会为 PR 添加 `lgtm` 标签并删除 `needs-*-more-lgtm` 标签; - 任何有效的 reviewer 或者 PR 作者进行 `/lgtm cancel` 都会重置 lgtm 计数。 - 阶段 2:人工批准 PR - - PR作者可向 PR 评论 `/assign`,这将推荐批准者,也可选择通知他们(例如:“pinging @foo for approval”)。 + - PR作者可向 PR 评论 `/assign`,这将推荐批准者,也可选择通知他们(例如:“pinging @foo for approval”)。 - 只有相关 OWNERS 文件中列出的人,无论是直接还是通过别名,如上所述,都可以充当批准人,包括打开 PR 的个人。 - 审批者寻找整体验收标准,包括与其他功能的依赖性、向前/向后兼容性、API 和标志定义等。 - 如果代码更改对他们来说很好,批准者会/approve输入 PR 评论或评论;如果他们改变主意,他们可以 `/approve cancel`