From 40489ede68364e24197f6eae49a835cd3a21698d Mon Sep 17 00:00:00 2001 From: "Erik Osterman (CEO @ Cloud Posse)" Date: Wed, 13 Mar 2024 23:01:10 -0500 Subject: [PATCH] Add more conditions for mergify config (#49) * add more checks * fix typo * fix typo * make mergify less error prone * update readme * clean up anchors and use active voice * comment do not close * address PR feedback --- .github/mergify.yml | 292 +++++++++++++++++++++++++++----------------- migrate/Makefile | 22 ---- migrate/README.md | 27 ++-- 3 files changed, 199 insertions(+), 142 deletions(-) diff --git a/.github/mergify.yml b/.github/mergify.yml index 077c1d98..1d1df6ad 100644 --- a/.github/mergify.yml +++ b/.github/mergify.yml @@ -2,20 +2,23 @@ # These are YAML anchor definitions to avoid repetition and make the file more readable shared: # Automated pull requests from bot users - automated_prs: &automated_prs - - "author=cloudpossebot" - - "author=github-actions[bot]" - - "author=dependabot[bot]" - - "author=renovate[bot]" + is_a_bot: &is_a_bot + - or: + - "author=cloudpossebot" + - "author=github-actions[bot]" + - "author=dependabot[bot]" + - "author=renovate[bot]" # Not a bot user not_a_bot: ¬_a_bot - - "-author=cloudpossebot" - - "-author=github-actions[bot]" - - "-author=dependabot[bot]" - - "-author=renovate[bot]" + - and: + - "-author=cloudpossebot" + - "-author=github-actions[bot]" + - "-author=dependabot[bot]" + - "-author=renovate[bot]" - external_contributor: &external_contributor + # Contribution is from an external contributor, not part of the organization + is_external_contributor: &is_external_contributor - and: - "-author=@engineering" - "-author=@contributors" @@ -23,42 +26,75 @@ shared: - "-author=@bots" - "-author=@approvers" - "-author=@security" + - and: *not_a_bot # Default branches - default_branch: &default_branch - - "base=main" - - "base=master" + is_default_branch: &is_default_branch + - and: + - "base=main" + - "base=master" # Release branches - release_branch: &release_branch - - "base~=^release/v\\d{1,2}$" + is_release_branch: &is_release_branch + - and: + - "base~=^release/v\\d{1,2}$" # Not a work in progress not_wip: ¬_wip - - "-title~=^(wip|WIP)" - - "-label~=(WIP|wip|do-not-merge|do not merge|triage|duplicate|invalid|stale|wontfix|triage|feedback)" - - '-draft' + - and: + - "-title~=^(wip|WIP)" + - "-label~=(WIP|wip|do-not-merge|do not merge|triage|stale|feedback|help needed)" + - '-draft' + + # Label indicates some action is needed + needs_attention: &needs_attention + - and: + - "label~=(triage|stale|feedback|help needed)" + + # Do not merge this PR + do_not_merge: &do_not_merge + - or: + - "label~=(do-not-merge|do not merge)" + - "title~=(do-not-merge|do not merge)" + + # Is a work in progress + is_wip: &is_wip + - or: + - "title~=^(wip|WIP)" + - "label~=(WIP|wip)" + - and: *do_not_merge + - and: *needs_attention + - 'draft' + + # Not in conflict + not_in_conflict: ¬_in_conflict + - and: + - "-conflict" + - "-label~=(conflict)" # Properly titled and described - pr_metadata: &pr_metadata - - "title~=(^[0-9A-Za-z]+)" - - body~=[0-9A-Za-z]{3,}\\s+[0-9A-Za-z]{3,}\\s+[0-9A-Za-z]{3,} - - -body~=(Describe high-level what changed) + pr_has_metadata: &pr_has_metadata + - and: + - "title~=(^[0-9A-Za-z]+)" + - body~=[0-9A-Za-z]{3,}\\s+[0-9A-Za-z]{3,}\\s+[0-9A-Za-z]{3,} + - -body~=(Describe high-level what changed) # Has reviews and no changes requested is_approved: &is_approved - - '#approved-reviews-by>=1' - - '#changes-requested-reviews-by>=1' - - "#review-threads-unresolved=0" - - "#commented-reviews-by=0" + - and: + - '#approved-reviews-by>=1' + - '#changes-requested-reviews-by=0' + - "#review-threads-unresolved=0" + - "#commented-reviews-by=0" # Checks are passing - checks_passing: &checks_passing - - "#check-pending=0" - - "#status-failure=0" + checks_are_passing: &checks_are_passing + - and: + - "#check-pending=0" + - "#status-failure=0" # Only for terraform files - require_terraform_checks_passing: &require_terraform_checks_passing + require_terraform_checks_are_passing: &require_terraform_checks_are_passing - or: - "-files~=\\.tf$" - and: @@ -66,22 +102,28 @@ shared: - "check-success=test/terratest" - -status-failure~=^(terratest|terraform)$ - require_codeowners_checks_passing: &require_codeowners_checks_passing + # CODEOWNERS check did not run or is not passing + require_codeowners_checks_are_passing: &require_codeowners_checks_are_passing - or: - "-files=CODEOWNERS" - and: - "check-success=validate-codeowners" + # Has changes to terraform code is_terraform: &is_terraform - - "files~=\\.tf$" + - and: + - "files~=\\.tf$" # It's not closed or merged is_open: &is_open - - -merged - - -closed + - and: + - -merged + - -closed + # Was recently updated one minute ago is_recent_commit: &is_recent_commit - - commits[*].date_committer > 1 minutes ago + - and: + - commits[*].date_committer > 1 minutes ago # README.md is updated together with README.yaml readme_updated: &readme_updated @@ -93,17 +135,22 @@ shared: - files=README.md - files=README.yaml + # Needs Cloud Posse review needs_cloudposse: &needs_cloudposse - or: - "files~=(mergify|settings|dependabot|renovate|CODEOWNERS|\\.github|Makefile|Dockerfile)" - "label~=(cloudposse)" + # Has no changes + has_no_changes: &has_no_changes + - and: + - "#files=0" # All the rules for the Pull Request pull_request_rules: - name: "label automated pull requests" conditions: - - or: *automated_prs + - and: *is_a_bot - and: *is_open actions: label: @@ -113,17 +160,17 @@ pull_request_rules: - name: "label automated pull requests that update readme" conditions: - and: *is_open - - or: *automated_prs + - and: *is_a_bot - "files=README.md" actions: label: - add: + toggle: - "readme" - name: "run terratest on automated pull requests that update terraform files" conditions: - and: *is_open - - or: *automated_prs + - and: *is_a_bot - and: *is_terraform actions: comment: @@ -132,10 +179,10 @@ pull_request_rules: - name: "merge automated PRs that only update the markdown files, images or videos" conditions: - and: *is_open + - and: *is_default_branch + - and: *is_a_bot - "#check-pending=0" - "head~=auto-update/.*" - - or: *default_branch - - or: *automated_prs - "files~=\\.(md|gif|png|jpg|mp4)$" actions: label: @@ -156,7 +203,7 @@ pull_request_rules: - "conflict" actions: comment: - message: "This pull request now has conflicts. Could you fix it @{{author}}? 🙏" + message: "💥 This pull request now has conflicts. Could you fix it @{{author}}? 🙏" label: toggle: - conflict @@ -184,6 +231,7 @@ pull_request_rules: - name: "ask to rebuild readme" conditions: + - and: *is_default_branch - and: *is_open - files=README.yaml - -files=README.md @@ -205,6 +253,7 @@ pull_request_rules: - name: "ask for title" conditions: + - and: *is_default_branch - and: *is_open - -title~=^[0-9A-Za-z]+ actions: @@ -217,6 +266,7 @@ pull_request_rules: - name: "ask for description" conditions: + - and: *is_default_branch - and: *is_open - -body~=[0-9A-Za-z]{3,}\\s+[0-9A-Za-z]{3,}\\s+[0-9A-Za-z]{3,} - body~=(Describe high-level what changed) @@ -231,12 +281,12 @@ pull_request_rules: - name: "remove outdated reviews" conditions: - and: *is_open - - or: *default_branch + - and: *is_default_branch actions: dismiss_reviews: changes_requested: true approved: true - message: "This Pull Request has been updated, so we're dismissing all reviews." + message: "This Pull Request was updated, so we're dismissing all reviews." - name: "remove triage label if approved" conditions: @@ -247,83 +297,95 @@ pull_request_rules: remove: - triage - #- name: label inactive PRs as stale - # conditions: - # - and: *is_open - # - commits[*].date_committer < 30 days ago - # actions: - # label: - # toggle: ["stale"] - - name: close automated PRs with persistent merge conflicts quickly conditions: - and: *is_open - - or: *automated_prs + - and: *is_a_bot - "conflict" - commits[*].date_committer < 1 days ago actions: close: message: | - This automated PR has been closed due to merge conflicts. + This automated PR was closed due to merge conflicts. - name: close stale PRs with merge conflicts conditions: - and: *is_open - "conflict" - commits[*].date_committer < 30 days ago + - updated-at < 7 days ago actions: close: message: | - This PR has been closed due to inactivity and merge conflicts. + This PR was closed due to inactivity and merge conflicts. 😭 Please resolve the conflicts and reopen if necessary. - #- name: close stale pull request after 90 days - # conditions: - # - and: *is_open - # - or: *default_branch - # - commits[*].date_committer < 90 days ago - # actions: - # close: - # message: | - # This pull request looks stale. Feel free to reopen it if you think it's a mistake. + - name: close stale pull request after 90 days + conditions: + - and: *is_open + - and: *is_default_branch + - commits[*].date_committer < 90 days ago + - updated-at < 3 days ago + - label~=(stale) + actions: + close: + message: | + 🚪 We're going close this pull request as it is now stale. Feel free to reopen it if you think it's a mistake. - #- name: close pull request waiting on feedback for 1 month - # conditions: - # - and: *is_open - # - or: *default_branch - # - or: - # - "label~=(feedback)" - # - '#commented-reviews-by > 0' - # - '#changes-requested-reviews-by > 0' - # - updated-at < 30 days ago - # actions: - # close: - # message: | - # We haven't heard back from you, so we're closing this pull request. - # Feel free to reopen it if you think it's a mistake. + - name: label stale pull request after 30 days + conditions: + - and: *is_open + - and: *is_default_branch + - commits[*].date_committer < 30 days ago + - updated-at < 7 days ago + - -label~=(stale|triage) + actions: + label: + toggle: + - stale + comment: + message: | + Heads up! This pull request looks stale. It will be closed soon, if there are no new commits. ⏳ - #- name: close pull request marked as invalid, duplicate or won't fix - # conditions: - # - and: *is_open - # - or: *default_branch - # - "label~=(duplicate|invalid|wontfix)" - # actions: - # close: - # message: | - # This pull request is no longer applicable. - # Feel free to reopen it if you think it's a mistake. + - name: close pull request waiting on feedback for 1 month + conditions: + - and: *is_open + - and: *is_default_branch + - "label~=(stale)" + - or: + - "label~=(feedback)" + - '#commented-reviews-by > 0' + - '#changes-requested-reviews-by > 0' + - updated-at < 30 days ago + actions: + close: + message: | + 📬 We haven't heard back from you, so we're closing this pull request. + Feel free to reopen it if you think it's a mistake. - #- name: close pull request that is a work in progress and in active for 1 month - # conditions: - # - and: *is_open - # - or: *default_branch - # - "label~=(WIP|wip|do-not-merge|do not merge)" - # - updated-at < 30 days ago - # actions: - # close: - # message: | - # This pull request was marked as a work in progress and looks abandoned. - # Feel free to reopen it if you think it's a mistake. + - name: close pull request marked as invalid, duplicate or won't fix + conditions: + - and: *is_open + - and: *is_default_branch + - "label~=(duplicate|invalid|wontfix)" + actions: + close: + message: | + ⚰️ This pull request is no longer applicable. + Feel free to reopen it if you think it's a mistake. + + - name: close pull request that is a work in progress and in active for 1 month + conditions: + - and: *is_open + - and: *is_default_branch + - and: *is_wip + - commits[*].date_committer < 90 days ago + - updated-at < 30 days ago + actions: + close: + message: | + This pull request was marked as a work in progress and looks abandoned. + Feel free to reopen it if you think it's a mistake. - name: remove certain labels on close conditions: @@ -336,19 +398,27 @@ pull_request_rules: - name: "close Pull Requests without files changed" conditions: - and: *is_open - - "#files=0" + - and: *has_no_changes actions: label: add: - "no-changes" close: - message: "This pull request has been automatically closed because there are no longer any changes." + message: | + This pull request was automatically closed as it no longer contains any changes. + + This typically happens when another merged pull request has already included this request's + proposed modifications into the default branch. - name: welcome new contributors conditions: - and: *is_open + - and: *not_wip - and: *not_a_bot - - and: *external_contributor + - and: *not_in_conflict + - and: *is_external_contributor + - and: *is_default_branch + - updated-at < 5 minutes ago actions: comment: message: | @@ -379,7 +449,7 @@ pull_request_rules: - name: Add needs-test label on new commits conditions: - and: *is_open - - or: *default_branch + - and: *is_default_branch - and: *is_terraform - and: *is_recent_commit - -label=~needs-test @@ -390,8 +460,8 @@ pull_request_rules: - name: Remove needs-test label when required tests pass conditions: - and: *is_open - - or: *default_branch - - and: *require_terraform_checks_passing + - and: *is_default_branch + - and: *require_terraform_checks_are_passing actions: label: remove: ["needs-test"] @@ -424,7 +494,7 @@ pull_request_rules: - name: rebase pull request when it's more than 10 commits behind main conditions: - and: *is_open - - or: *default_branch + - and: *is_default_branch - "#commits-behind>=10" actions: rebase: @@ -441,18 +511,18 @@ pull_request_rules: #- name: mergeable # conditions: # - and: *is_open - # - or: *default_branch + # - and: *is_default_branch # actions: # post_check: # success_conditions: # - and: # - and: *not_wip - # - and: *pr_metadata + # - and: *pr_has_metadata # - and: *readme_updated # - and: *is_approved - # - and: *checks_passing - # - and: *require_terraform_checks_passing - # - and: *require_codeowners_checks_passing + # - and: *checks_are_passing + # - and: *require_terraform_checks_are_passing + # - and: *require_codeowners_checks_are_passing # title: | # {% if check_status == "success" %} # This PR is ready to be merged diff --git a/migrate/Makefile b/migrate/Makefile index 41c7ee09..538ade7e 100644 --- a/migrate/Makefile +++ b/migrate/Makefile @@ -1,26 +1,4 @@ -export SHELL := /bin/bash -export TMPDIR := ./tmp -export MIGRATE_PATH := $(PWD) -export MIGRATION_PATH := $(MIGRATE_PATH)/migrations/$(MIGRATION) - deps: brew install gh yq pip3 install yamlfix -clean:: - rm -rf $(TMPDIR)/git-xargs* - -.PHONY: run -run: - export GITHUB_OAUTH_TOKEN=$$(gh auth token); \ - echo $$GITHUB_OAUTH_TOKEN; \ - mkdir -p $$TMPDIR; \ - git-xargs \ - --loglevel DEBUG \ - --skip-archived-repos \ - --repos $(MIGRATION_PATH)/repos.txt \ - --keep-cloned-repositories \ - --branch-name migration/$(MIGRATION) \ - --commit-message 'chore: run migration/$(MIGRATION)' \ - --skip-pull-requests \ - '$(MIGRATE_PATH)/run.sh' '$(MIGRATION)' diff --git a/migrate/README.md b/migrate/README.md index 6203b78f..b1cad924 100644 --- a/migrate/README.md +++ b/migrate/README.md @@ -3,41 +3,50 @@ Create a migration script in [`migrations//script.sh`](migrations/) and use the helpers in [`lib/`](lib/). ```shell -make run MIGRATION=20240302 +# Export this environment variable to actually create and merge PRs +XARGS_DRY_RUN=false + +# Pass the migration and the list of repos in the migration to process. +./git-xargs.sh 20240304 repos.txt ``` ## How it Works -The `lib/` folder contains helper libraries (bash). Define one file per tool. See the existing libraies for -conventions. +The `lib/` folder contains helper libraries (bash). Define one file per tool. See the existing libraries for conventions. The `migrations/` folder contains subfolders, one per migration. By convention, we should use dates (e.g. `20230302`). Inside each date-based migration folder, there should be a few files. - `README.md` which will both describe the migration and be used as the body of the PR. -- `repos.txt` a list of repos which this migration applies to -- `script.sh` a script that is invoked in the context of the library, and will perform the migration operations +- `repos.txt` a list of repos which this migration applies to. You can also batch this using the `split` command. +- `script.sh` a script that is invoked in the context of the library and will perform the migration operations There's a `templates/` folder, which contains a number of subfolders. Each subfolder represents a repository type. Including a `default` folder, which is used when no files are found for a given repository type. ## Helper Functions -- The `template_file` function will use the `REPO_TYPE` environment variable to find the best template file. It searches from the most specific to the list specific (e.g., `defaults/``). -- The `info` function emits a friendly message. +- The `template_file` function will use the `REPO_TYPE` environment variable to find the best template file. It searches from the most specific to the list specific (e.g., `defaults/``) +- The `info` function emits a friendly message - The `error` function emits the error and exits 1 - The `title` function sets the title that will be used subsequently when the PR is opened - The `install` function will use the `template_file` function to install a file from one of the suitable templates +- THe `gh` function will call the `gh` command but respect GitHub API rate limits ## Tips & Tricks -1. Use `yq` for manipulating YAML. It will preserve comments, but not whitespace. It will also replace unicode characters with their escape sequence. Use `yamlfix` to restore the unicode character. +1. Use `yq` for manipulating YAML. It will preserve comments but not whitespace. It will also replace Unicode characters with their escape sequence. Use `yamlfix` to restore the Unicode character. 2. Use `yamlfix` to format YAML and normalize whitespace. +
Past Limitations Using `git-xargs` CLI + ## Notable Limitations +We encountered significant obstacles using `git-xargs`. Here they are for posterity. + - `git-xargs` cannot add labels -- `git-xargs` [ignores `.gitignore`](https://github.com/gruntwork-io/git-xargs/issues/53), so it's best to handle clean up before exiting the script +- `git-xargs` [ignores `.gitignore`](https://github.com/gruntwork-io/git-xargs/issues/53), so it's best to handle clean-up before exiting the script - `git-xargs` will not update PR title/description, so it's advisable to just use `gh` CLI instead - `git-xargs` cannot auto-merge, so use `gh-cli` in script to commit, push, open PR, then merge - Using `gh-cli` to bypass the `git-xargs` deficiencies, means rate limiting isn't respected by `git-xargs` +
\ No newline at end of file