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

[FIX]: ignore-sensitive-files-pr check #2701

Conversation

JiyaGupta-cs
Copy link

@JiyaGupta-cs JiyaGupta-cs commented Nov 19, 2024

What kind of change does this PR introduce?

ignore-sensitive-files-pr check works but that if the action has on: [pull_request], the label(s) need to be there at PR creation time and not added afterward otherwise your github action doesn't rerun.

Default triggers are openedsynchronize and reopened
Runs your workflow when activity on a pull request in the workflow's repository occurs. For example, if no activity types are specified, the workflow runs when a pull request is opened or reopened or when the head branch of the pull request is updated.

Doc Reference: https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#pull_request

So to fix this added types: [opened, synchronize, reopened, labeled, unlabeled] which triggers the workflows when these actions take place

Other Informmation-
Also fixed : persist-crendetials to persist-credentials

Issue Number:

Fixes #2699

Have you read the contributing guide?

Yes

Summary by CodeRabbit

  • New Features

    • Enhanced pull request workflow with additional trigger events for better automation.
    • New validation step to ensure source and target branches of pull requests are not the same.
  • Bug Fixes

    • Corrected credential handling options for improved security during workflow execution.
    • Implemented file count check to prevent excessive changes in pull requests.
  • Improvements

    • Reformatted conditions for better readability and clarity in the workflow processes.

Copy link

coderabbitai bot commented Nov 19, 2024

Walkthrough

The changes in this pull request modify the GitHub Actions workflow in .github/workflows/pull-request.yml. The pull_request trigger now includes additional event types, enhancing when the workflow executes. Corrections were made to the persist-crendetials option across multiple jobs, which is now consistently named persist-credentials. New validations were added to check if the source and target branches of pull requests are the same, and the File-count-check job now limits the number of changed files to 100. Minor adjustments were also made to the JSDocs job for boolean evaluation.

Changes

File Path Change Summary
.github/workflows/pull-request.yml - Added event types to pull_request trigger.
- Corrected persist-crendetials to persist-credentials in multiple jobs.
- Added branch validation step in Code-Quality-Checks.
- Reformatted conditional in Check-Sensitive-Files job.
- Implemented file count check in File-count-check job.
- Changed JSDocs condition from 'True' to 'true'.

Assessment against linked issues

Objective Addressed Explanation
GitHub Action should ignore changes in sensitive files with the label (2699) The changes do not implement the logic to skip checks based on the ignore-sensitive-files-pr label.

Possibly related issues

Possibly related PRs

Suggested labels

ignore-sensitive-files-pr

Suggested reviewers

  • palisadoes
  • varshith257

Poem

🐇 In the meadow where workflows play,
Changes made to brighten the day.
Branches checked and files in line,
With credentials fixed, all will be fine!
A hop, a skip, let errors cease,
In our code, we find our peace! 🌼


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

Our Pull Request Approval Process

We have these basic policies to make the approval process smoother for our volunteer team.

Testing Your Code

Please make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:

  1. The overall code coverage drops below the target threshold of the repository
  2. Any file in the pull request has code coverage levels below the repository threshold
  3. Merge conflicts

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.

Reviewers

Do not assign reviewers. Our Queue Monitors will review your PR and assign them.
When your PR has been assigned reviewers contact them to get your code reviewed and approved via:

  1. comments in this PR or
  2. our slack channel

Reviewing Your Code

Your reviewer(s) will have the following roles:

  1. arbitrators of future discussions with other contributors about the validity of your changes
  2. point of contact for evaluating the validity of your work
  3. person who verifies matching issues by others that should be closed.
  4. person who gives general guidance in fixing your tests

CONTRIBUTING.md

Read our CONTRIBUTING.md file. Most importantly:

  1. PRs with issues not assigned to you will be closed by the reviewer
  2. Fix the first comment in the PR so that each issue listed automatically closes

Other

  1. 🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise.
  2. Read the CONTRIBUTING.md file make

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.

Copy link

@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 (1)
.github/workflows/pull-request.yml (1)

95-97: Consider case-insensitive label matching

The label check contains(github.event.pull_request.labels.*.name, 'ignore-sensitive-files-pr') is case-sensitive. This could cause issues if the label is added with different casing.

Consider using a case-insensitive comparison or documenting the exact label name required:

!contains(toJSON(github.event.pull_request.labels.*.name), 'ignore-sensitive-files-pr')
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 66d7632 and 18bd625.

📒 Files selected for processing (1)
  • .github/workflows/pull-request.yml (5 hunks)
🔇 Additional comments (3)
.github/workflows/pull-request.yml (3)

15-15: LGTM: Comprehensive trigger types added

The addition of labeled and unlabeled events to the workflow triggers directly addresses the issue of label detection after PR creation. The complete set of triggers ensures the workflow runs at all necessary points in the PR lifecycle.


315-315: LGTM: Boolean comparison fixed

The change from 'True' to 'true' aligns with standard shell script and YAML boolean conventions.


31-31: Verify all instances of persist-credentials are fixed

The typo fix from persist-crendetials to persist-credentials is applied in multiple locations. Let's ensure we haven't missed any instances.

Also applies to: 105-105, 160-160

✅ Verification successful

All persist-credentials configurations are correctly spelled

The verification shows that all three instances in pull-request.yml are using the correct spelling persist-credentials: true. No typos or incorrect variations were found in the workflow files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining typos of 'persist-crendetials' in workflow files
echo "Checking for any remaining typos in persist-credentials..."
rg -i "persist-crendetials" .github/workflows/

Length of output: 173


Script:

#!/bin/bash
# Let's check both the correct spelling and potential typos to get a complete picture
echo "Checking all instances of persist-credentials (both correct and incorrect spellings)..."
rg -i "persist-cr[ea]n?d[ea]n?tials" .github/workflows/ -A 1

Length of output: 582

coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 19, 2024
@varshith257 varshith257 added the ignore-sensitive-files-pr The contributor has a legitimate reason for editiing protected files label Nov 19, 2024
@varshith257 varshith257 changed the base branch from main to develop November 19, 2024 12:29
@varshith257 varshith257 dismissed coderabbitai[bot]’s stale review November 19, 2024 12:29

The base branch was changed.

@varshith257 varshith257 changed the base branch from develop to main November 19, 2024 12:29
@varshith257
Copy link
Member

@JiyaGupta-cs can you push changes for develop-postgres branch? It seems you have made changes in main branch.

@JiyaGupta-cs
Copy link
Author

@JiyaGupta-cs can you push changes for develop-postgres branch? It seems you have made changes in main branch.

ignore-sensitive-files-pr check this is being triggered from main branch itself
The develop-postgres branch does not have it
So that's why I have pushed that in the main itself

@varshith257
Copy link
Member

varshith257 commented Nov 19, 2024

Submit for develop too, for main branch, the develop branch will get merged into main for every 3 weeks

@varshith257
Copy link
Member

@coderabbitai resolve

Copy link

coderabbitai bot commented Nov 19, 2024

✅ Actions performed

Comments resolved and changes approved.

@varshith257
Copy link
Member

The workflow still failing, check it once

if: ${{ github.actor != 'dependabot[bot]' && !contains(github.event.pull_request.labels.*.name, 'ignore-sensitive-files-pr') }}
if: >-
github.actor != 'dependabot[bot]' &&
!contains(github.event.pull_request.labels.*.name, 'ignore-sensitive-files-pr')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
!contains(github.event.pull_request.labels.*.name, 'ignore-sensitive-files-pr')
(github.event.pull_request.labels | map(.name) | contains('ignore-sensitive-files-pr') | not)

I think the issue is related of some syntax errors where the label is not read from PR. ONCE check with map and contains something like above

Copy link
Author

Choose a reason for hiding this comment

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

let me try with normal syntax

@palisadoes
Copy link
Contributor

Please submit your PRs against our develop-postgres branch. The default is main, so you will have to modify this before submitting your PR for review. PRs made against main will be closed. Closing.

@palisadoes palisadoes closed this Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-sensitive-files-pr The contributor has a legitimate reason for editiing protected files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ignore-sensitive-files-pr check isn't skipping check when label is added
3 participants