-
Notifications
You must be signed in to change notification settings - Fork 167
feat: Add support for checking all changes of PR #767
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
base: main-enterprise
Are you sure you want to change the base?
feat: Add support for checking all changes of PR #767
Conversation
This Adds support for checking entire PR context instead of only the last commit. Also fixes a bunch of errors making sure it uses the correct Repo when looping over the list. When setting `RepoConfig` it Assigned the object to the last one in the loop of Repositories. Resulting in wrong comparision.
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.
PR Overview
This PR adds support for checking an entire pull request’s changes rather than only the last commit and fixes a bug with repo configuration assignment. Key changes include:
- Introducing a new environment flag (PR_USE_BASE_SHA) to control base SHA selection in check runs.
- Fixing the assignment of RepoConfig to avoid unintended mutation by using a new object copy.
- Updating debug logging in rulesets and settings to improve clarity during processing.
Reviewed Changes
File | Description |
---|---|
index.js | Adjusts SHA selection logic based on the new PR_USE_BASE_SHA flag for check runs. |
lib/env.js | Introduces the new environment variable PR_USE_BASE_SHA with a default value of 'false'. |
lib/plugins/rulesets.js | Updates logging to use the repository owner for clarity when fetching rulesets. |
lib/settings.js | Adds debug logs at the start of sync routines, fixes the RepoConfig assignment to avoid mutation, and refines the filtering of results in nop mode. |
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (3)
lib/settings.js:306
- Creating a new object for repoConfig prevents mutating the original configuration, which avoids side effects in later processing. Please verify that all downstream consumers correctly handle the new object reference.
repoConfig = Object.assign({}, repoConfig, { name: repo.repo, org: repo.owner })
lib/settings.js:867
- [nitpick] Defaulting to an empty array when res is not an array might discard valid result data; confirm that res is always expected to be an array in nop mode.
const results = Array.isArray(res) ? res.flat(3).filter(r => r) : [];
lib/plugins/rulesets.js:31
- [nitpick] Using 'this.repo.owner' for logging improves clarity regarding the organization context; ensure that this change is consistent with how the organization is referenced throughout the code.
this.log.debug(`Getting all rulesets for the org ${this.repo.owner}`)
This Adds support for checking entire PR context instead of only the last commit.
Also fixes a bunch of errors making sure it uses the correct Repo when looping over the list. When setting
RepoConfig
it Assigned the object to the last one in the loop of Repositories. Resulting in wrong comparison.Log of the RepoConfig error: