From 519e9e1ae8797627d4a1fb0e80b5b420c78a3f72 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Tue, 28 Jan 2020 22:52:49 +0200 Subject: [PATCH] ci: minor PullApprove fixes/improvements (#35015) This is a follow-up to #34814 to fix some typos in patterns and make them more similar to the old patterns from `.github/CODEOWNERS`. PR Close #35015 --- .pullapprove.yml | 107 ++++++++++++++++++-------------------- docs/TRIAGE_AND_LABELS.md | 3 ++ scripts/github/merge-pr | 4 +- 3 files changed, 56 insertions(+), 58 deletions(-) diff --git a/.pullapprove.yml b/.pullapprove.yml index 8caa39d88e..ada936d3d4 100644 --- a/.pullapprove.yml +++ b/.pullapprove.yml @@ -5,6 +5,9 @@ #################################################################################### # # Configuration of code ownership and review approvals for the angular/angular repo. +# +# More info: https://docs.pullapprove.com/ +# # ========================================================= # General rules / philosophy # ========================================================= @@ -73,6 +76,8 @@ # ========================================================= # @angular/framework-global-approvers # ========================================================= +# Used for approving minor changes, large-scale refactorings, and in emergency situations. +# # IgorMinar # josephperrott # kara @@ -81,9 +86,11 @@ # ========================================================= # @angular/framework-global-approvers-for-docs-only-changes # ========================================================= +# Used for approving minor documentation-only changes that don't require engineering review. +# # aikidave -# kapunahelewong # gkalpak +# kapunahelewong # petebacondarwin @@ -162,7 +169,7 @@ groups: conditions: - > contains_any_globs(files, [ - 'packages/compiler-cli/ngcc/*' + 'packages/compiler-cli/ngcc/**' ]) reviewers: users: @@ -175,23 +182,6 @@ groups: - ~framework-global-approvers-for-docs-only-changes - # ========================================================= - # Framework: Compiler + CLI integration - # ========================================================= - fw-cli-integration: - conditions: - - > - contains_any_globs(files, [ - 'packages/compiler-cli/src/ngtools/**', - ]) - reviewers: - users: - - filipesilva - teams: - - ~framework-global-approvers - - ~framework-global-approvers-for-docs-only-changes - - # ========================================================= # Framework: Core # ========================================================= @@ -298,7 +288,7 @@ groups: 'aio/content/images/guide/structural-directives/**', 'aio/content/guide/user-input.md', 'aio/content/examples/user-input/**', - 'aio/content/images/guide/user-input/*' + 'aio/content/images/guide/user-input/**' ]) reviewers: users: @@ -324,7 +314,7 @@ groups: 'packages/examples/http/**', 'aio/content/guide/http.md', 'aio/content/examples/http/**', - 'aio/content/images/guide/http/*' + 'aio/content/images/guide/http/**' ]) reviewers: users: @@ -379,7 +369,7 @@ groups: 'aio/content/images/guide/dynamic-form/**', 'aio/content/guide/reactive-forms.md', 'aio/content/examples/reactive-forms/**', - 'aio/content/images/guide/reactive-forms/*' + 'aio/content/images/guide/reactive-forms/**' ]) reviewers: users: @@ -411,7 +401,7 @@ groups: 'packages/compiler-cli/src/extract_i18n.ts', 'packages/localize/**', 'aio/content/guide/i18n.md', - 'aio/content/examples/i18n/*' + 'aio/content/examples/i18n/**' ]) reviewers: users: @@ -432,7 +422,7 @@ groups: contains_any_globs(files, [ 'packages/platform-server/**', 'aio/content/guide/universal.md', - 'aio/content/examples/universal/*' + 'aio/content/examples/universal/**' ]) reviewers: users: @@ -454,7 +444,7 @@ groups: 'packages/examples/router/**', 'aio/content/guide/router.md', 'aio/content/examples/router/**', - 'aio/content/images/guide/router/*' + 'aio/content/images/guide/router/**' ]) reviewers: users: @@ -480,7 +470,7 @@ groups: 'aio/content/guide/service-worker-config.md', 'aio/content/guide/service-worker-devops.md', 'aio/content/guide/service-worker-intro.md', - 'aio/content/images/guide/service-worker/*' + 'aio/content/images/guide/service-worker/**' ]) reviewers: users: @@ -512,7 +502,7 @@ groups: 'aio/content/guide/upgrade-performance.md', 'aio/content/guide/upgrade-setup.md', 'aio/content/guide/ajs-quick-reference.md', - 'aio/content/examples/ajs-quick-reference/*' + 'aio/content/examples/ajs-quick-reference/**' ]) reviewers: users: @@ -530,15 +520,15 @@ groups: conditions: - > contains_any_globs(files, [ - 'testing/**', + '**/testing/**', 'aio/content/guide/testing.md', 'aio/content/examples/testing/**', - 'aio/content/images/guide/testing/*' + 'aio/content/images/guide/testing/**' ]) reviewers: users: - - kara - IgorMinar + - kara - pkozlowski-opensource teams: - ~framework-global-approvers @@ -558,7 +548,7 @@ groups: 'packages/platform-browser/src/security/**', 'aio/content/guide/security.md', 'aio/content/examples/security/**', - 'aio/content/images/guide/security/*' + 'aio/content/images/guide/security/**' ]) reviewers: users: @@ -581,9 +571,9 @@ groups: ]) reviewers: users: - - kyliau - IgorMinar - josephperrott + - kyliau teams: - ~framework-global-approvers - ~framework-global-approvers-for-docs-only-changes @@ -598,8 +588,8 @@ groups: contains_any_globs(files, [ 'packages/language-service/**', 'aio/content/guide/language-service.md', - 'aio/content/images/guide/language-service/*' - ]) + 'aio/content/images/guide/language-service/**' + ]) reviewers: users: - kyliau @@ -650,7 +640,7 @@ groups: conditions: - > contains_any_globs(files, [ - 'integration/*' + 'integration/**' ]) reviewers: users: @@ -660,7 +650,6 @@ groups: - mhevery teams: - ~framework-global-approvers - - ~framework-global-approvers-for-docs-only-changes # ========================================================= @@ -684,7 +673,7 @@ groups: 'aio/content/examples/getting-started-v0/**', 'aio/content/examples/getting-started/**', 'aio/content/start/**', - 'aio/content/images/guide/start/*' + 'aio/content/images/guide/start/**' ]) reviewers: users: @@ -733,7 +722,7 @@ groups: 'aio/content/guide/practical-observable-usage.md', 'aio/content/examples/practical-observable-usage/**', 'aio/content/guide/rx-library.md', - 'aio/content/examples/rx-library/*' + 'aio/content/examples/rx-library/**' ]) reviewers: users: @@ -782,17 +771,18 @@ groups: conditions: - > contains_any_globs(files, [ - 'aio/content/guide/typescript-configuration.md', - 'aio/content/examples/setup/**', - 'aio/content/guide/build.md', - 'aio/content/images/guide/build/**', - 'aio/content/guide/cli-builder.md', - 'aio/content/guide/deployment.md', - 'aio/content/images/guide/deployment/**', - 'aio/content/guide/file-structure.md', - 'aio/content/guide/ivy.md', - 'aio/content/guide/web-worker.md' - 'aio/content/guide/workspace-config.md', + 'aio/content/guide/typescript-configuration.md', + 'aio/content/examples/setup/**', + 'aio/content/guide/build.md', + 'aio/content/images/guide/build/**', + 'aio/content/guide/cli-builder.md', + 'aio/content/examples/cli-builder/**', + 'aio/content/guide/deployment.md', + 'aio/content/images/guide/deployment/**', + 'aio/content/guide/file-structure.md', + 'aio/content/guide/ivy.md', + 'aio/content/guide/web-worker.md' + 'aio/content/guide/workspace-config.md', ]) reviewers: users: @@ -866,7 +856,7 @@ groups: 'aio/content/examples/docs-style-guide/**', 'aio/content/images/guide/docs-style-guide/**', 'aio/content/guide/visual-studio-2015.md', - 'aio/content/examples/visual-studio-2015/*' + 'aio/content/examples/visual-studio-2015/**' ]) reviewers: users: @@ -886,24 +876,29 @@ groups: - > contains_any_globs(files, [ '*', - '.buildkite/**', '.circleci/**', '.devcontainer/**', '.github/**', '.vscode/**', + '.yarn/**', 'docs/BAZEL.md', 'packages/*', 'packages/examples/test-utils/**', 'packages/private/**', 'scripts/**', 'third_party/**', + 'tools/brotli-cli/**', + 'tools/browsers/**', 'tools/build/**', + 'tools/circular_dependency_test/**', 'tools/gulp-tasks/**', + 'tools/ng_rollup_bundle/**', 'tools/ngcontainer/**', 'tools/npm/**', - 'tools/public_api_guard/BUILD', + 'tools/public_api_guard/BUILD.bazel', 'tools/public_api_guard/public_api_guard.bzl', 'tools/rxjs/**', + 'tools/saucelabs/**', 'tools/size-tracking/**', 'tools/source-map-test/**', 'tools/symbol-extractor/**', @@ -913,8 +908,8 @@ groups: 'tools/validate-commit-message/**', 'tools/yarn/**', 'tools/*', - '*.bzl', - '*.BAZEL' + '**/*.bzl', + '**/*.bazel' ]) reviewers: users: @@ -934,7 +929,7 @@ groups: conditions: - > contains_any_globs(files, [ - 'tools/material-ci/*' + 'tools/components-repo-ci/**' ]) reviewers: users: @@ -976,7 +971,7 @@ groups: contains_any_globs(files, [ 'aio/scripts/_payload-limits.json', 'integration/_payload-limits.json' - ]) + ]) reviewers: users: - IgorMinar diff --git a/docs/TRIAGE_AND_LABELS.md b/docs/TRIAGE_AND_LABELS.md index d04fe546c9..81cc61d97b 100644 --- a/docs/TRIAGE_AND_LABELS.md +++ b/docs/TRIAGE_AND_LABELS.md @@ -173,6 +173,9 @@ Before a PR can be merged it must be approved by the appropriate reviewer(s). To ensure that the right people review each change, we set review requests using [PullApprove](https://https://docs.pullapprove.com/) (via `.pullapprove`) and require that each PR has at least one approval from an appropriate code owner. +If the PR author is a code owner themselves, the approval can come from _any_ repo collaborator (person with write access). +In any case, the reviewer should actually look through the code and provide feedback if necessary. + Note that approved state does not mean a PR is ready to be merged. For example, a reviewer might approve the PR but request a minor tweak that doesn't need further review, e.g., a rebase or small uncontroversial change. Only the `PR action: merge` label means that the PR is ready for merging. diff --git a/scripts/github/merge-pr b/scripts/github/merge-pr index 4ad7a09aa1..5d81c093bf 100755 --- a/scripts/github/merge-pr +++ b/scripts/github/merge-pr @@ -131,8 +131,8 @@ CHERRY_PICK_PR="git cherry-pick merge_pr_base..merge_pr" # # This check is used to enforce that we don't merge PRs that have not been rebased recently and could result in merging # of non-approved or otherwise bad changes. -REQUIRED_BASE_SHA_MASTER="a03a9236f2aed5d00012d25f032aa43a046d91da" # pullapprove => CODEOWNERS migration -REQUIRED_BASE_SHA_PATCH="a03a9236f2aed5d00012d25f032aa43a046d91da" # pullapprove => CODEOWNERS migration +REQUIRED_BASE_SHA_MASTER="296dc0622f0e8c4e803ff4f19a5c6fe02a2ae66e" # CODEOWNERS => PullApprove migration +REQUIRED_BASE_SHA_PATCH="110f6c91b904819cab639861b54b6a989e176942" # CODEOWNERS => PullApprove migration if [[ $MERGE_MASTER == 1 ]]; then REQUIRED_BASE_SHA="$REQUIRED_BASE_SHA_MASTER" # check patch only if patch-only PR