From cae9eaf8b2c1412fcd17d5bdab2ee2aae7ce1cd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Saray=20Cabrera=20Padr=C3=B3n?= Date: Thu, 7 Mar 2024 16:17:13 +0100 Subject: [PATCH 1/2] Drop isolated_migrations GitHub action --- .github/workflows/isolated_migrations.yml | 87 ----------------------- 1 file changed, 87 deletions(-) delete mode 100644 .github/workflows/isolated_migrations.yml diff --git a/.github/workflows/isolated_migrations.yml b/.github/workflows/isolated_migrations.yml deleted file mode 100644 index 7ce3d4e7694..00000000000 --- a/.github/workflows/isolated_migrations.yml +++ /dev/null @@ -1,87 +0,0 @@ -# GitHub action that checks: -# - that the migrations and the related code changes are not in the same pull request -# - that the schema.rb and data_schema.rb files are included in the pull request that introduces the corresponding migration -# Based on https://github.com/marketplace/actions/changed-files - -name: Isolated Migrations - -on: - pull_request - -jobs: - check_changed_files: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - - name: Group files that have changed - id: changed-files-yaml - uses: tj-actions/changed-files@v42 - with: - files_yaml: | - linters: - - '.github/workflows/isolated_migrations.yml' - - 'src/api/.rubocop*.yml' - migrations: - - src/api/db/migrate/** - - src/api/db/schema.rb - - src/api/db/data/** - - src/api/db/data_schema.rb - not_migrations: - - '!src/api/db/migrate/**' - - '!src/api/db/schema.rb' - - '!src/api/db/data/**' - - '!src/api/db/data_schema.rb' - - '!src/api/db/attribute_descriptions.rb' - - '!src/api/db/seeds.rb' - - '!.github/workflows/isolated_migrations.yml' - - '!src/api/.rubocop*.yml' - schema_migrations: - - src/api/db/migrate/** - data_migrations: - - src/api/db/data/** - schema: - - src/api/db/schema.rb - data_schema: - - src/api/db/data_schema.rb - - - name: List the files related to linters that this PR changes - if: steps.changed-files-yaml.outputs.linters_any_changed == 'true' - run: | - for file in ${{ steps.changed-files-yaml.outputs.linters_all_changed_files }}; do - echo "- $file" - done - - - name: List the files related to migrations that this PR changes - if: steps.changed-files-yaml.outputs.migrations_any_changed == 'true' - run: | - for file in ${{ steps.changed-files-yaml.outputs.migrations_all_changed_files }}; do - echo "- $file" - done - - - name: List the files not related to migrations that this PR changes - if: steps.changed-files-yaml.outputs.not_migrations_any_changed == 'true' - run: | - for file in ${{ steps.changed-files-yaml.outputs.not_migrations_all_changed_files }}; do - echo "- $file" - done - - - name: Check if the Pull Request contains code changes together with migrations - if: (steps.changed-files-yaml.outputs.migrations_any_changed == 'true') && (steps.changed-files-yaml.outputs.not_migrations_all_changed_files_count > 0) - run: | - echo "There are code changes together with migrations. Please split them into two Pull Requests" - exit 1 - - - name: Missing schema changes - if: (steps.changed-files-yaml.outputs.schema_migrations_any_changed == 'true') && (steps.changed-files-yaml.outputs.linters_any_changed == 'false') && (steps.changed-files-yaml.outputs.schema_any_changed == 'false') - run: | - echo "You introduced some schema migration, please introduce the schema.rb changes as well" - exit 1 - - - name: Missing data schema changes - if: (steps.changed-files-yaml.outputs.data_schema_migrations_any_changed == 'true') && (steps.changed-files-yaml.outputs.linters_any_changed == 'false') && (steps.changed-files-yaml.outputs.data_schema_any_changed == 'false') - run: | - echo "You introduced some data migrations, please introduce the data_schema.rb changes as well" - exit 1 From 571fe45d49cf118d420d8c9cc0f369bf72d022a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Saray=20Cabrera=20Padr=C3=B3n?= Date: Thu, 14 Mar 2024 23:03:56 +0100 Subject: [PATCH 2/2] Add GitHub actions to warn about migrations They write comments when: - there is a migration together with other changes (except schema changes), - there is a migration but not its corresponding db schema, - there is a data migration but not its corresponding data schema. It is splitted into two actions, the first one stores the texts in artifacts (text files) and the second one create the comments using those texts. This is required to allow pull requests that come from forks to write. --- .../set_warnings_about_migrations.yml | 67 +++++++++++++++++++ .github/workflows/warn_about_migrations.yml | 48 +++++++++++++ 2 files changed, 115 insertions(+) create mode 100644 .github/workflows/set_warnings_about_migrations.yml create mode 100644 .github/workflows/warn_about_migrations.yml diff --git a/.github/workflows/set_warnings_about_migrations.yml b/.github/workflows/set_warnings_about_migrations.yml new file mode 100644 index 00000000000..7da2ac66c78 --- /dev/null +++ b/.github/workflows/set_warnings_about_migrations.yml @@ -0,0 +1,67 @@ +name: Set warnings about migrations + +on: + pull_request + +permissions: + contents: read + +jobs: + comment_on_pr: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Group files that have changed + id: changed-files-yaml + uses: tj-actions/changed-files@v43 + with: + files_yaml: | + migrations: + - src/api/db/migrate/** + - src/api/db/data/** + not_migrations: + - '!src/api/db/migrate/**' + - '!src/api/db/data/**' + - '!src/api/db/schema.rb' + - '!src/api/db/data_schema.rb' + db_migrations: + - src/api/db/migrate/** + data_migrations: + - src/api/db/data/** + db_schema: + - src/api/db/schema.rb + data_schema: + - src/api/db/data_schema.rb + + - name: Store PR number as artifact + run: | + mkdir ./artifacts + echo ${{ github.event.number }} > ./artifacts/pr_number.txt + + - name: Store warning text about migrations + if: (steps.changed-files-yaml.outputs.migrations_any_changed == 'true') && (steps.changed-files-yaml.outputs.not_migrations_all_changed_files_count > 0) + run: | + COMMENT_TEXT_MIGRATIONS=":warning: Please make sure the migration is shipped in an independent Pull Request. :warning:"$'\n' + COMMENT_TEXT_MIGRATIONS+=":heavy_check_mark: You can include schema changes, annotations and validations for consistency but :x: avoid committing other changes with it."$'\n' + echo "$COMMENT_TEXT_MIGRATIONS" > ./artifacts/comment_text_migrations.txt + + - name: Store warning text about missing db schema + if: (steps.changed-files-yaml.outputs.db_migrations_any_changed == 'true') && (steps.changed-files-yaml.outputs.db_schema_any_changed == 'false') + run: | + COMMENT_TEXT_DB_SCHEMA=":warning: There is a db migration but not a db schema. Please commit it."$'\n' + echo "$COMMENT_TEXT_DB_SCHEMA" > ./artifacts/comment_text_db_schema.txt + + - name: Store warning text about missing data schema + if: (steps.changed-files-yaml.outputs.data_migrations_any_changed == 'true') && (steps.changed-files-yaml.outputs.data_schema_any_changed == 'false') + run: | + COMMENT_TEXT_DATA_SCHEMA=":warning: There is a data migration but not a data schema. Please commit it."$'\n' + echo "$COMMENT_TEXT_DATA_SCHEMA" > ./artifacts/comment_text_data_schema.txt + + - name: Upload artifacts + uses: actions/upload-artifact@v4 + with: + name: migrations_artifacts + path: artifacts/ diff --git a/.github/workflows/warn_about_migrations.yml b/.github/workflows/warn_about_migrations.yml new file mode 100644 index 00000000000..ddbbd78c1f0 --- /dev/null +++ b/.github/workflows/warn_about_migrations.yml @@ -0,0 +1,48 @@ +name: Warn about migrations +on: + workflow_run: + workflows: ['Set warnings about migrations'] + types: + - completed +permissions: + contents: read +jobs: + comment_on_pr: + runs-on: ubuntu-latest + permissions: + pull-requests: write + steps: + - name: Download artifacts + uses: dawidd6/action-download-artifact@v3 + with: + workflow: ${{ github.event.workflow_run.workflow_id }} + workflow_conclusion: success + + - name: Fetch PR number from artifacts + run: | + echo "pr_number=$(cat migrations_artifacts/pr_number.txt)" >> $GITHUB_ENV + + - name: Add comment about migration to PR + uses: thollander/actions-comment-pull-request@v2 + if: ${{ hashFiles('migrations_artifacts/comment_text_migrations.txt') != '' }} + with: + filePath: migrations_artifacts/comment_text_migrations.txt + pr_number: ${{ env.pr_number }} + comment_tag: comment_about_migrations + + - name: Add comment about missing db schema to PR + uses: thollander/actions-comment-pull-request@v2 + if: ${{ hashFiles('migrations_artifacts/comment_text_db_schema.txt') != '' }} + with: + filePath: migrations_artifacts/comment_text_db_schema.txt + pr_number: ${{ env.pr_number }} + comment_tag: comment_about_db_schema + + - name: Add comment about missing data schema to PR + uses: thollander/actions-comment-pull-request@v2 + if: ${{ hashFiles('migrations_artifacts/comment_text_data_schema.txt') != '' }} + with: + filePath: migrations_artifacts/comment_text_data_schema.txt + pr_number: ${{ env.pr_number }} + comment_tag: comment_about_data_schema +