Added Merge Conflict Check #3138
Conversation
WalkthroughA new GitHub Actions job called Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/pull-request.yml (3)
453-463: Add workflow dependencies for better integrationThe merge conflict check should run early in the workflow to fail fast and prevent unnecessary builds. Consider adding this job as a dependency for other jobs.
Add the following to integrate with the workflow:
Merge-Conflict-Check: name: Check for Merge Conflicts runs-on: ubuntu-latest + needs: [] # Run early in the workflow + if: github.actor != 'dependabot[bot]' # Consistent with other jobs steps:Then update other jobs to depend on this check:
Code-Quality-Checks: name: Performs linting, formatting, type-checking... runs-on: ubuntu-latest + needs: [Merge-Conflict-Check] steps:
452-452: Consider job placement in workflowThe merge conflict check is added at the end of the workflow file. Consider moving it closer to the beginning, near other early validation jobs like
Check-Target-Branch, to maintain a logical grouping of validation checks.
453-479: Add documentation for the merge conflict checkConsider adding documentation about this new workflow job:
- Update the workflow file's header comments to mention this check
- Add a section in the PR guidelines (PR_GUIDELINES.md) about merge conflict detection
🧰 Tools
🪛 yamllint (1.35.1)
[error] 465-465: trailing spaces
(trailing-spaces)
[error] 479-479: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 465-465: trailing spaces
(trailing-spaces)
[error] 479-479: no new line character at the end of file
(new-line-at-end-of-file)
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/pull-request.yml (1)
Line range hint
12-16: Add workflow trigger for target branch updatesThe current workflow trigger configuration doesn't automatically re-run when the target branch is updated. This can lead to stale merge conflict status.
Add
pull_request_targetevent or modify the existing trigger:on: pull_request: branches: - '**' + types: + - opened + - reopened + - synchronize + - ready_for_review + pull_request_target: + types: + - synchronized🧰 Tools
🪛 yamllint (1.35.1)
[error] 461-461: trailing spaces
(trailing-spaces)
[error] 475-475: no new line character at the end of file
(new-line-at-end-of-file)
🧹 Nitpick comments (1)
.github/workflows/pull-request.yml (1)
461-461: Fix YAML formatting issuesThere are two formatting issues to address:
- Remove trailing spaces
- Add newline at end of file
Apply this diff:
- run: | + run: | PR_NUMBER=${{ github.event.pull_request.number }} # ... rest of the script ... - fi - shell: bash \ No newline at end of file + fi + shell: bash +Also applies to: 475-475
🧰 Tools
🪛 yamllint (1.35.1)
[error] 461-461: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 461-461: trailing spaces
(trailing-spaces)
[error] 475-475: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (2)
.github/workflows/pull-request.yml (2)
457-459: LGTM!The checkout step is correctly implemented using the latest version of the action.
460-475: Improve error handling and reliabilityThe implementation needs improvements for error handling and retry mechanism.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 461-461: trailing spaces
(trailing-spaces)
[error] 475-475: no new line character at the end of file
(new-line-at-end-of-file)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3138 +/- ##
=====================================================
+ Coverage 20.93% 89.64% +68.70%
=====================================================
Files 301 322 +21
Lines 7685 8451 +766
Branches 1678 1900 +222
=====================================================
+ Hits 1609 7576 +5967
+ Misses 5977 646 -5331
- Partials 99 229 +130 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/pull-request.yml (3)
18-25: Align pull_request_target events with pull_request eventsThe
pull_request_targettrigger only listens forsynchronizeevents, whilepull_requestlistens for multiple events. Consider aligning them to ensure consistent merge conflict detection across all PR events.pull_request_target: types: + - opened + - reopened - synchronize + - ready_for_review
470-505: Improve shell script style and error handlingWhile the script logic is solid, we can improve the shell scripting style:
- Check curl exit code directly instead of using $?
- Use heredoc for better script readability
- name: Check Mergeable Status via API - run: | - PR_NUMBER=${{ github.event.pull_request.number }} - max_retries=3 - retry_delay=5 - - for ((i=1; i<=max_retries; i++)); do - echo "Attempt $i of $max_retries" - - response=$(curl -s -f -H "Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}" \ - "https://api.github.com/repos/${{ github.repository }}/pulls/$PR_NUMBER") - - if [ $? -ne 0 ]; then + run: | + cat << 'EOF' > check_mergeable.sh + #!/bin/bash + set -euo pipefail + + PR_NUMBER=$1 + GITHUB_TOKEN=$2 + GITHUB_REPOSITORY=$3 + max_retries=3 + retry_delay=5 + + for ((i=1; i<=max_retries; i++)); do + echo "Attempt $i of $max_retries" + + if ! response=$(curl -s -f -H "Authorization: Bearer ${GITHUB_TOKEN}" \ + "https://api.github.com/repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}"); then echo "Failed to call GitHub API" if [ $i -eq $max_retries ]; then exit 1 fi sleep $retry_delay continue - fi - - mergeable=$(echo "$response" | jq -r '.mergeable') - if [ "$mergeable" == "true" ]; then + fi + + if mergeable=$(echo "$response" | jq -r '.mergeable'); then + if [ "$mergeable" == "true" ]; then echo "No conflicts detected." exit 0 - elif [ "$mergeable" == "false" ]; then + elif [ "$mergeable" == "false" ]; then echo "Merge conflicts detected." exit 1 - else + else echo "Mergeable status unknown." if [ $i -eq $max_retries ]; then exit 1 fi sleep $retry_delay - fi + fi + else + echo "Failed to parse mergeable status" + exit 1 + fi done + EOF + + chmod +x check_mergeable.sh + ./check_mergeable.sh \ + "${{ github.event.pull_request.number }}" \ + "${{ secrets.GITHUB_TOKEN }}" \ + "${{ github.repository }}"🧰 Tools
🪛 actionlint (1.7.4)
471-471: shellcheck reported issue in this script: SC2181:style:11:8: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
🪛 yamllint (1.35.1)
[error] 471-471: trailing spaces
(trailing-spaces)
[error] 475-475: trailing spaces
(trailing-spaces)
[error] 478-478: trailing spaces
(trailing-spaces)
[error] 481-481: trailing spaces
(trailing-spaces)
[error] 490-490: trailing spaces
(trailing-spaces)
[error] 505-505: no new line character at the end of file
(new-line-at-end-of-file)
471-505: Fix YAML formatting issuesThere are several YAML formatting issues that should be fixed:
- Remove trailing spaces
- Add newline at end of file
- run: | + run: | PR_NUMBER=${{ github.event.pull_request.number }} max_retries=3 retry_delay=5 - + for ((i=1; i<=max_retries; i++)); do echo "Attempt $i of $max_retries" - + response=$(curl -s -f -H "Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}" \ "https://api.github.com/repos/${{ github.repository }}/pulls/$PR_NUMBER") - + if [ $? -ne 0 ]; then echo "Failed to call GitHub API" if [ $i -eq $max_retries ]; then exit 1 fi sleep $retry_delay continue fi - + mergeable=$(echo "$response" | jq -r '.mergeable') # ... rest of the script ... done +🧰 Tools
🪛 actionlint (1.7.4)
471-471: shellcheck reported issue in this script: SC2181:style:11:8: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
🪛 yamllint (1.35.1)
[error] 471-471: trailing spaces
(trailing-spaces)
[error] 475-475: trailing spaces
(trailing-spaces)
[error] 478-478: trailing spaces
(trailing-spaces)
[error] 481-481: trailing spaces
(trailing-spaces)
[error] 490-490: trailing spaces
(trailing-spaces)
[error] 505-505: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pull-request.yml
471-471: shellcheck reported issue in this script: SC2181:style:11:8: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/pull-request.yml
[error] 471-471: trailing spaces
(trailing-spaces)
[error] 475-475: trailing spaces
(trailing-spaces)
[error] 478-478: trailing spaces
(trailing-spaces)
[error] 481-481: trailing spaces
(trailing-spaces)
[error] 490-490: trailing spaces
(trailing-spaces)
[error] 505-505: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
.github/workflows/pull-request.yml (1)
461-466: LGTM! Job configuration follows project conventionsThe job configuration is well-structured and consistent with other jobs in the workflow:
- Runs on ubuntu-latest
- Skips dependabot PRs
- Appropriate dependency on Code-Quality-Checks
noman2002
added
the
ignore-sensitive-files-pr
noman2002
merged commit e31085a
into
PalisadoesFoundation:develop-postgres
What kind of change does this PR introduce?
Feature
Issue Number:
Fixes #3015
Did you add tests for your changes?
No
Snapshots/Videos:
If relevant, did you update the documentation?
No
Summary
Added the Merge Conflict Check in
.github/workflows/pull-request.ymlDoes this PR introduce a breaking change?
No
Other information
Have you read the contributing guide?
Yes
Summary by CodeRabbit