-
-
Notifications
You must be signed in to change notification settings - Fork 712
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
update pull-request.yml to ignore actions on ignore-sensitive-files-pr #2384
update pull-request.yml to ignore actions on ignore-sensitive-files-pr #2384
Conversation
WalkthroughThe pull request modifies the GitHub Actions workflow in the 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 ProcessWe have these basic policies to make the approval process smoother for our volunteer team. Testing Your CodePlease make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:
The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing. 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
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2384 +/- ##
========================================
Coverage 97.99% 97.99%
========================================
Files 246 246
Lines 7020 7020
Branches 2015 2015
========================================
Hits 6879 6879
Misses 131 131
Partials 10 10 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this 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 (1)
.github/workflows/pull-request.yml (1)
Line range hint 279-289
: Consider enhancing the job name and error message.
The job correctly implements the target branch validation, but could benefit from these improvements:
- Check-Target-Branch:
+ Enforce-Develop-Target-Branch:
if: ${{ github.actor != 'dependabot[bot]' }}
- name: Check Target Branch
+ name: Ensures PRs target the develop branch
runs-on: ubuntu-latest
steps:
- name: Check if the target branch is develop
if: github.event.pull_request.base.ref != 'develop'
run: |
- echo "Error: Pull request target branch must be 'develop'. Please refer PR_GUIDELINES.md"
+ echo "Error: Invalid target branch '${GITHUB_BASE_REF}'. All pull requests must target the 'develop' branch as outlined in PR_GUIDELINES.md"
+ echo "Current target: ${GITHUB_BASE_REF}"
+ echo "Required target: develop"
exit 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/pull-request.yml (1 hunks)
🔇 Additional comments (1)
.github/workflows/pull-request.yml (1)
83-83
: LGTM! Verify the condition works as expected.
The condition correctly implements the new label-based exclusion while maintaining the existing dependabot check.
Let's verify the condition works as expected:
✅ Verification successful
Let me gather more information about how these sensitive files are being checked in the workflows.
Let me check how the sensitive files are being validated in the job to ensure the condition is properly integrated with the actual checks.
Let me check what happens after the files are detected to ensure the complete workflow is properly handling the condition.
Condition is correctly implemented and properly integrated with the sensitive files check
The verification confirms that:
- The condition is properly integrated with the
tj-actions/changed-files
action that checks for sensitive file modifications - The job correctly skips for both dependabot and when the 'ignore-sensitive-files-pr' label is present
- The list of protected files is comprehensive and properly monitored
- The workflow fails appropriately when unauthorized changes are detected (through the echo and implicit failure)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that sensitive file checks are properly configured
# Expected: The job should run by default, skip for dependabot, and skip when label is present
# Test 1: Check if any other workflows reference these sensitive files
rg -l "env\.example|\.node-version|\.husky|scripts/|package\.json|tsconfig\.json|\.gitignore|\.eslintrc\.json|\.eslintignore|\.prettierrc|\.prettierignore|vite\.config\.ts|docker-compose\.yaml|Dockerfile|CODEOWNERS|LICENSE|setup\.ts|\.coderabbit\.yaml|CODE_OF_CONDUCT\.md|CODE_STYLE\.md|CONTRIBUTING\.md|DOCUMENTATION\.md|INSTALLATION\.md|ISSUE_GUIDELINES\.md|PR_GUIDELINES\.md|README\.md" .github/workflows/
# Test 2: Check for any other conditions that might override this check
rg "ignore-sensitive-files-pr" .github/workflows/
Length of output: 867
Script:
#!/bin/bash
# Check the implementation of sensitive file checks in pull-request.yml
rg -A 10 "Check-Sensitive-Files" .github/workflows/pull-request.yml
# Check if there are any other jobs or conditions in other workflows that might conflict
rg -A 5 "Check-Sensitive-Files|sensitive.*files" .github/workflows/
Length of output: 1218
Script:
#!/bin/bash
# Get the complete Check-Sensitive-Files job implementation
rg -B 2 -A 30 "Check-Sensitive-Files:" .github/workflows/pull-request.yml
# Check if there are any other places where these files are being validated
rg "tj-actions/changed-files" .github/workflows/
Length of output: 1389
Script:
#!/bin/bash
# Get the complete job implementation including what happens after files are detected
rg -B 5 -A 40 "Get Changed Unauthorized files" .github/workflows/pull-request.yml
Length of output: 1477
@varshith257 can you please help me why this "PR Workflow / Runs Introspection on the GitHub talawa-api repo on the schema.graphql file (pull_req" is failing? |
Please fix the introspection tests. That will be needed to determine whether this PR will cause the desired behavior. You should ask to be assigned to this issue too and make the changes a part of this PR. Here are some examples of issues fixing this in the past: |
@palisadoes sir , Do you want me to solve issue #2385 in this pr or i can create a separate pr ? |
@govindsingh3477 Your wish but the aim is to make all workflows green here in the PR. I think you can push here |
This issue arises due to createGroupChat , createDirectChat ,createMessageChat were removed in commit PalisadoesFoundation/talawa-api@2f605f9, now i should remove these from talwa admin ? |
See here |
But I think Chat feature worked by @disha1202. Let's confirm from her |
i think these can be removed as some related functions were removed in |
Closing in favor of #2391 |
label
What kind of change does this PR introduce?
feature
Issue Number:
Fixes #2383
Did you add tests for your changes?
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Other information
Have you read the contributing guide?
yes
Summary by CodeRabbit