Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: auto revert PRs when they break main #17061

Merged
merged 31 commits into from
Nov 28, 2024
Merged

ci: auto revert PRs when they break main #17061

merged 31 commits into from
Nov 28, 2024

Conversation

busla
Copy link
Member

@busla busla commented Nov 28, 2024

Revert PRs on label

Support PR revert by setting the revert label.

Manual revert

Setting the revert label automates the creation of a revert PR.

Automated revert

When our checksuit fails on main the revert label is set on the merged PR automatically that triggers the revert process.

Feature flag

The revert feature is disabled by default but can be enabled by updating the following github variable. With the flag disabled the workflow lifecycle will be triggered but the actual PR revert will not be created.

ENABLE_PR_REVERT=true

TODO

  • dismiss the reviewers for automerge to work.
  • notify the author of the revert.

Summary by CodeRabbit

  • New Features

    • Introduced a new GitHub Actions workflow to automate the process of reverting pull requests based on workflow outcomes.
    • Added scripts to facilitate the retrieval of pull request numbers and automate the reverting process.
  • Bug Fixes

    • Updated error handling in the shellcheck job of the pull request workflow to improve linting accuracy.
  • Documentation

    • Enhanced clarity in the workflow configurations for better understanding and maintenance.

@busla busla requested a review from a team as a code owner November 28, 2024 00:07
Copy link
Contributor

coderabbitai bot commented Nov 28, 2024

Walkthrough

A new GitHub Actions workflow named revert-pr.yaml has been added to automate the reversion of merged pull requests that fail checks. This workflow triggers on specific events such as pull request closures and check suite completions. It includes jobs that detect failures, create revert pull requests, and allow for manual reverts, labeling them accordingly. Additionally, two new scripts, revert-pr.sh and get-pr.sh, have been introduced to facilitate the API interactions necessary for reverting pull requests and retrieving pull request numbers associated with commits.

Changes

File Change Summary
.github/workflows/revert-pr.yaml Added a new workflow to automate reverting pull requests that fail checks after merging.
scripts/ci/revert-pr.sh Introduced a script to automate the reversion of a pull request using GitHub's API.
scripts/ci/get-pr.sh Added a script to retrieve the pull request number associated with a specific commit.
.github/workflows/pullrequest.yml Updated the run-shellcheck job's configuration to change error handling from fail_on_error to fail-level.

Possibly related PRs

Suggested labels

deploy-feature, high priority


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Outside diff range and nitpick comments (2)
scripts/ci/revert-pr.sh (1)

1-22: Consider architectural improvements for better maintainability

To improve the script's maintainability and integration with the workspace:

  1. Move configuration to environment variables or a config file:

    • GitHub repository name
    • Revert PR title format
    • Error messages
  2. Add logging levels and structured output:

    • Use a logging function for consistent output
    • Support different verbosity levels
    • Consider JSON output format for better parsing
  3. Integration with workspace:

    • Consider using workspace-level utilities for consistent behavior
    • Add documentation about script usage in CI context

Would you like me to provide an example implementation of these improvements?

.github/workflows/revert-pr.yaml (1)

3-14: Consider optimizing workflow triggers

The current trigger configuration might cause redundant workflow executions due to overlapping events. Consider:

  1. Using only workflow_run event to catch all failures
  2. Specifying critical workflows instead of '*' to reduce unnecessary executions
 on:
-  pull_request:
-    types: [closed]
-    branches:
-      - main
-  check_suite:
-    types: [completed]
   workflow_run:
-    workflows: ['*']
+    workflows: ['CI', 'Tests']  # specify your critical workflows
     types: [completed]
     branches:
       - main
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between edf13e2 and 0bcbeb2.

📒 Files selected for processing (2)
  • .github/workflows/revert-pr.yaml (1 hunks)
  • scripts/ci/revert-pr.sh (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
scripts/ci/revert-pr.sh (1)

Pattern scripts/**/*: "Confirm that the code adheres to the following:

  • Script efficiency, readability, and maintainability.
  • Usage of environmental variables and configuration files for flexibility.
  • Integration with NX workspace utilities and commands."
🪛 actionlint (1.7.4)
.github/workflows/revert-pr.yaml

22-22: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


30-30: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (2)
.github/workflows/revert-pr.yaml (2)

31-31: Verify the revert-pr.sh script implementation

The workflow depends on ./scripts/ci/revert-pr.sh which needs to be reviewed for proper error handling and GitHub API interaction.

✅ Verification successful

Script has proper error handling and GitHub API interaction

The revert-pr.sh script implementation is well-structured and includes:

  • Proper usage of GitHub CLI (gh) for API interactions
  • Error handling through command substitution (read -r)
  • GraphQL mutation with proper parameter handling
  • Returns the new PR number for workflow continuation
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the revert script exists and review its implementation
if [[ -f "./scripts/ci/revert-pr.sh" ]]; then
  cat "./scripts/ci/revert-pr.sh"
else
  echo "Warning: revert-pr.sh script not found"
  # Search for the script in case the path is different
  fd "revert-pr.sh"
fi

Length of output: 608


22-23: Verify custom runner configuration and enhance conditions

  1. The ec2-runners label needs to be properly configured in your self-hosted runners setup
  2. The condition might miss edge cases where the PR is merged but the failure is from a different workflow

Consider enhancing the conditions:

-    if: github.event.pull_request.merged == true && github.event.workflow_run.conclusion == 'failure'
+    if: |
+      github.event.workflow_run.conclusion == 'failure' &&
+      github.event.workflow_run.head_branch == 'main' &&
+      contains(github.event.workflow_run.head_commit.message, 'Merge pull request')
🧰 Tools
🪛 actionlint (1.7.4)

22-22: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)

scripts/ci/revert-pr.sh Show resolved Hide resolved
scripts/ci/revert-pr.sh Show resolved Hide resolved
scripts/ci/revert-pr.sh Show resolved Hide resolved
.github/workflows/revert-pr.yaml Outdated Show resolved Hide resolved
@busla busla marked this pull request as draft November 28, 2024 06:57
Copy link

codecov bot commented Nov 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 35.69%. Comparing base (10e9499) to head (eb745c1).
Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #17061   +/-   ##
=======================================
  Coverage   35.69%   35.69%           
=======================================
  Files        6938     6938           
  Lines      147122   147122           
  Branches    41824    41824           
=======================================
  Hits        52516    52516           
  Misses      94606    94606           
Flag Coverage Δ
web 2.44% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10e9499...eb745c1. Read the comment docs.

@datadog-island-is
Copy link

datadog-island-is bot commented Nov 28, 2024

Datadog Report

Branch report: ci/revert-pr-workflow
Commit report: 6596aa6
Test service: web

✅ 0 Failed, 84 Passed, 0 Skipped, 31.36s Total Time
➡️ Test Sessions change in coverage: 1 no change

@busla busla added the revert label Nov 28, 2024
@busla busla added revert and removed revert labels Nov 28, 2024
@busla busla added revert Will trigger a revert PR and removed revert-manual labels Nov 28, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (4)
.github/workflows/revert-pr.yaml (4)

1-1: Fix typo in workflow name

The workflow name "Checksuit" appears to be a typo. Consider changing it to "Checksuite" for better clarity and consistency.

-name: Checksuit
+name: Checksuite

43-50: Enhance PR fetching error handling

The PR fetching logic could be improved with better error handling and logging.

-          COMMIT_SHA="${{ github.event.workflow_run.head_commit.id }}"
-          PR_NUMBER=$(bash ./scripts/ci/get-pr.sh "$COMMIT_SHA")
-
-          if [ -n "$PR_NUMBER" ]; then
-            echo "PR_NUMBER=$PR_NUMBER" >> $GITHUB_OUTPUT
-          else
-            echo "No merged PR found for commit $COMMIT_SHA."
-          fi
+          COMMIT_SHA="${{ github.event.workflow_run.head_commit.id }}"
+          if [ -z "$COMMIT_SHA" ]; then
+            echo "Error: Commit SHA is empty"
+            exit 1
+          fi
+          
+          if ! PR_NUMBER=$(bash ./scripts/ci/get-pr.sh "$COMMIT_SHA"); then
+            echo "Error: Failed to fetch PR number for commit $COMMIT_SHA"
+            exit 1
+          fi
+          
+          if [ -n "$PR_NUMBER" ]; then
+            echo "Found PR #$PR_NUMBER for commit $COMMIT_SHA"
+            echo "PR_NUMBER=$PR_NUMBER" >> $GITHUB_OUTPUT
+          else
+            echo "No merged PR found for commit $COMMIT_SHA"
+          fi

86-86: Improve condition readability

The condition is complex and could be split for better readability.

-    if: ${{ github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'revert') && (github.event.pull_request.merged == true || github.event.pull_request.draft == true) && github.actor != 'github-actions' }}
+    if: |
+      ${{
+        github.event_name == 'pull_request' &&
+        contains(github.event.pull_request.labels.*.name, 'revert') &&
+        (github.event.pull_request.merged == true || github.event.pull_request.draft == true) &&
+        github.actor != 'github-actions'
+      }}

1-98: Consider enhancing the revert workflow

The workflow architecture could benefit from these improvements:

  1. Add notifications to PR authors when their PRs are reverted
  2. Implement a cooldown period before reverting to allow for quick fixes
  3. Add detailed logging of revert reasons in PR comments

Would you like assistance in implementing any of these suggestions?

🧰 Tools
🪛 actionlint (1.7.4)

28-28: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


42-42: shellcheck reported issue in this script: SC2086:info:5:34: Double quote to prevent globbing and word splitting

(shellcheck)


70-70: property "fetch-pr" is not defined in object type {detect-failure: {outputs: {pr_number: string}; result: string}}

(expression)


74-74: property "fetch-pr" is not defined in object type {detect-failure: {outputs: {pr_number: string}; result: string}}

(expression)


80-80: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)


96-96: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between e71ba87 and 1fc93e2.

📒 Files selected for processing (1)
  • .github/workflows/revert-pr.yaml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/revert-pr.yaml

28-28: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


42-42: shellcheck reported issue in this script: SC2086:info:5:34: Double quote to prevent globbing and word splitting

(shellcheck)


70-70: property "fetch-pr" is not defined in object type {detect-failure: {outputs: {pr_number: string}; result: string}}

(expression)


74-74: property "fetch-pr" is not defined in object type {detect-failure: {outputs: {pr_number: string}; result: string}}

(expression)


80-80: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)


96-96: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (4)
.github/workflows/revert-pr.yaml (4)

70-70: ⚠️ Potential issue

Fix incorrect job reference

The condition references needs.fetch-pr but the job is named detect-failure.

-    if: ${{ needs.fetch-pr.outputs.PR_NUMBER && github.event.workflow_run.conclusion == 'failure' }}
+    if: ${{ needs.detect-failure.outputs.PR_NUMBER && github.event.workflow_run.conclusion == 'failure' }}

Likely invalid or redundant comment.

🧰 Tools
🪛 actionlint (1.7.4)

70-70: property "fetch-pr" is not defined in object type {detect-failure: {outputs: {pr_number: string}; result: string}}

(expression)


97-98: ⚠️ Potential issue

Fix shell script safety issues in manual revert

The shell commands need proper quoting and error handling.

-          REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "${{ github.event.pull_request.number }}")
-          gh pr edit $REVERT_PR --add-label "automerge,high priority"
+          if ! REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "${{ github.event.pull_request.number }}"); then
+            echo "Error: Failed to create revert PR"
+            exit 1
+          fi
+          gh pr edit "$REVERT_PR" --add-label "automerge" --add-label "high priority"

Likely invalid or redundant comment.


81-82: ⚠️ Potential issue

Fix shell script safety issues

The shell commands need proper quoting to prevent word splitting and potential issues.

-          REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "$PR_NUMBER")
-          gh pr edit $REVERT_PR --add-label "automerge,high priority"
+          if ! REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "$PR_NUMBER"); then
+            echo "Error: Failed to create revert PR"
+            exit 1
+          fi
+          gh pr edit "$REVERT_PR" --add-label "automerge" --add-label "high priority"

Likely invalid or redundant comment.


28-28: Verify the custom runner label

The workflow uses a custom runner label ec2-runners. Please ensure this label is properly configured in your GitHub Actions settings or self-hosted runners configuration.

🧰 Tools
🪛 actionlint (1.7.4)

28-28: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (2)
.github/workflows/revert-pr.yaml (2)

1-1: Fix typo in workflow name

The workflow name "Checksuit" appears to be a typo. Consider changing it to "Checksuite" for better clarity and consistency.

-name: Checksuit
+name: Checksuite

86-86: Improve condition readability

The job condition is complex and could benefit from being split into multiple conditions for better readability.

-    if: ${{ github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'revert') && (github.event.pull_request.merged == true || github.event.pull_request.draft == true) && github.actor != 'github-actions' }}
+    if: |
+      ${{
+        github.event_name == 'pull_request' &&
+        contains(github.event.pull_request.labels.*.name, 'revert') &&
+        (github.event.pull_request.merged == true || github.event.pull_request.draft == true) &&
+        github.actor != 'github-actions'
+      }}
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 1fc93e2 and d07330a.

📒 Files selected for processing (1)
  • .github/workflows/revert-pr.yaml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/revert-pr.yaml

28-28: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


42-42: shellcheck reported issue in this script: SC2086:info:5:34: Double quote to prevent globbing and word splitting

(shellcheck)


80-80: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)


96-96: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (2)
.github/workflows/revert-pr.yaml (2)

80-82: 🛠️ Refactor suggestion

Improve script execution and label handling

The current implementation has potential issues:

  1. No error handling for the revert script
  2. Label addition uses a comma-separated string which is fragile
-          REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "$PR_NUMBER")
-          gh pr edit $REVERT_PR --add-label "automerge,high priority"
+          if ! REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "$PR_NUMBER"); then
+            echo "Failed to create revert PR"
+            exit 1
+          fi
+          gh pr edit "$REVERT_PR" --add-label "automerge" --add-label "high priority"
🧰 Tools
🪛 actionlint (1.7.4)

80-80: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)


96-98: 🛠️ Refactor suggestion

Apply same improvements as create-revert-pr job

The script execution and label handling have the same issues as the create-revert-pr job.

-          REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "${{ github.event.pull_request.number }}")
-          gh pr edit $REVERT_PR --add-label "automerge,high priority"
+          if ! REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "${{ github.event.pull_request.number }}"); then
+            echo "Failed to create revert PR"
+            exit 1
+          fi
+          gh pr edit "$REVERT_PR" --add-label "automerge" --add-label "high priority"

Likely invalid or redundant comment.

🧰 Tools
🪛 actionlint (1.7.4)

96-96: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/revert-pr.yaml Outdated Show resolved Hide resolved
.github/workflows/revert-pr.yaml Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (2)
.github/workflows/revert-pr.yaml (2)

1-1: Rename workflow to better reflect its purpose

The name "Checksuit" is unclear. Consider renaming to something more descriptive like "PR Auto-Revert" or "Main Branch Protection - Auto-Revert".

-name: Checksuit
+name: PR Auto-Revert

91-91: Improve readability of job condition

The condition is complex and hard to read. Consider breaking it down using environment variables or job outputs.

-    if: ${{ github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'revert') && (github.event.pull_request.merged == true || github.event.pull_request.draft == true) && github.actor != 'github-actions' }}
+    if: |
+      ${{
+        github.event_name == 'pull_request' &&
+        contains(github.event.pull_request.labels.*.name, 'revert') &&
+        (github.event.pull_request.merged == true || github.event.pull_request.draft == true) &&
+        github.actor != 'github-actions'
+      }}
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between d07330a and 1491be0.

📒 Files selected for processing (1)
  • .github/workflows/revert-pr.yaml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/revert-pr.yaml

28-28: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


42-42: shellcheck reported issue in this script: SC2086:info:5:34: Double quote to prevent globbing and word splitting

(shellcheck)


71-71: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


82-82: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)


88-88: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


100-100: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (4)
.github/workflows/revert-pr.yaml (4)

21-23: LGTM: Environment setup is secure

The GitHub token is properly configured using secrets.


28-28: Document custom runner label ec2-runners

The ec2-runners label is used across the workflow but lacks documentation. Add documentation in one of these locations:

  • Create a README.md in .github/workflows/ directory explaining the custom runner setup
  • Add a comment in the workflow files where this label is used
  • Create an actionlint configuration file to validate this custom label
🧰 Tools
🪛 actionlint (1.7.4)

28-28: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


82-85: 🛠️ Refactor suggestion

Improve script execution and label handling

The current implementation has several potential issues:

  1. No error handling for the revert script
  2. Label addition uses a comma-separated string which is fragile
-          REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "$PR_NUMBER")
-          gh pr edit $REVERT_PR --add-label "automerge,high priority"
+          if ! REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "$PR_NUMBER"); then
+            echo "Failed to create revert PR"
+            exit 1
+          fi
+          gh pr edit "$REVERT_PR" --add-label "automerge" --add-label "high priority"
🧰 Tools
🪛 actionlint (1.7.4)

82-82: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)


100-102: 🛠️ Refactor suggestion

Fix script execution and label handling

Similar to the create-revert-pr job, improve error handling and label management.

-          REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "${{ github.event.pull_request.number }}")
-          gh pr edit $REVERT_PR --add-label "automerge,high priority"
+          if ! REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "${{ github.event.pull_request.number }}"); then
+            echo "Failed to create revert PR"
+            exit 1
+          fi
+          gh pr edit "$REVERT_PR" --add-label "automerge" --add-label "high priority"

Likely invalid or redundant comment.

🧰 Tools
🪛 actionlint (1.7.4)

100-100: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/revert-pr.yaml Outdated Show resolved Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@busla
Copy link
Member Author

busla commented Nov 28, 2024

@coderabbitai resolve

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (3)
.github/workflows/revert-pr.yaml (3)

1-1: Consider a more descriptive workflow name

The name "Checksuit" doesn't clearly convey the workflow's purpose of reverting PRs. Consider renaming to something more descriptive like "PR Auto-Revert" or "PR Revert Handler".

-name: Checksuit
+name: PR Auto-Revert Workflow

94-94: Improve condition readability

The complex condition could be split into multiple lines for better readability.

-    if: ${{ github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'revert') && (github.event.pull_request.merged == true || github.event.pull_request.draft == true) && github.actor != 'github-actions' }}
+    if: |
+      ${{
+        github.event_name == 'pull_request' &&
+        contains(github.event.pull_request.labels.*.name, 'revert') &&
+        (github.event.pull_request.merged == true || github.event.pull_request.draft == true) &&
+        github.actor != 'github-actions'
+      }}

21-22: Add documentation for environment variables

Document the ENABLE_PR_REVERT variable and consider adding notifications to alert PR authors when their PRs are being reverted.

Add a comment block explaining the configuration:

env:
  # GitHub token for API operations
  GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
  # Note: ENABLE_PR_REVERT must be set to 'true' to enable PR reversion
  # This can be configured in repository variables
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 1491be0 and 69c3943.

📒 Files selected for processing (1)
  • .github/workflows/revert-pr.yaml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/revert-pr.yaml

28-28: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


42-42: shellcheck reported issue in this script: SC2086:info:8:34: Double quote to prevent globbing and word splitting

(shellcheck)


74-74: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


85-85: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)


91-91: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


103-103: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (3)
.github/workflows/revert-pr.yaml (3)

28-28: Document custom runner label ec2-runners

The ec2-runners label is used but not documented. Add documentation in the workflow configuration.

🧰 Tools
🪛 actionlint (1.7.4)

28-28: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)


85-87: 🛠️ Refactor suggestion

Improve script execution and label handling

The current implementation needs better error handling and label management.

-          REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "$PR_NUMBER")
-          gh pr edit $REVERT_PR --add-label "automerge,high priority"
+          if ! REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "$PR_NUMBER"); then
+            echo "Failed to create revert PR"
+            exit 1
+          fi
+          gh pr edit "$REVERT_PR" --add-label "automerge" --add-label "high priority"

Likely invalid or redundant comment.

🧰 Tools
🪛 actionlint (1.7.4)

85-85: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)


103-105: 🛠️ Refactor suggestion

Add error handling for revert operation

Similar to the automated revert job, add proper error handling for the script execution and label addition.

-          REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "${{ github.event.pull_request.number }}")
-          gh pr edit $REVERT_PR --add-label "automerge,high priority"
+          if ! REVERT_PR=$(bash ./scripts/ci/revert-pr.sh "${{ github.event.pull_request.number }}"); then
+            echo "Failed to create revert PR"
+            exit 1
+          fi
+          gh pr edit "$REVERT_PR" --add-label "automerge" --add-label "high priority"

Likely invalid or redundant comment.

🧰 Tools
🪛 actionlint (1.7.4)

103-103: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting

(shellcheck)

Copy link
Contributor

coderabbitai bot commented Nov 28, 2024

✅ Actions performed

Comments resolved and changes approved.

@busla busla added automerge Merge this PR as soon as all checks pass and removed automerge Merge this PR as soon as all checks pass labels Nov 28, 2024
@kodiakhq kodiakhq bot merged commit a40be29 into main Nov 28, 2024
34 checks passed
@kodiakhq kodiakhq bot deleted the ci/revert-pr-workflow branch November 28, 2024 17:10
@busla busla added revert Will trigger a revert PR and removed revert Will trigger a revert PR labels Nov 28, 2024
This was referenced Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this PR as soon as all checks pass revert Will trigger a revert PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants