Conversation
There was a problem hiding this comment.
Pull request overview
Removes Copilot Code Review (CCR)-specific branching from the CodeQL Action and cleans up associated tests and PR-check infrastructure, aligning “managed/dynamic” workflow handling with the remaining supported modes.
Changes:
- Delete CCR detection (
isCCR) and related test helpers/tests. - Simplify feature-flag initialization to only use offline defaults when not running on GitHub.com (e.g., GHES), and adjust tests accordingly.
- Remove CCR PR-check template/workflow and update the PR template wording to reflect current managed workflow products.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/testing-utils.ts | Removes the CCR test helper (mockCCR) and its now-unused import. |
| src/feature-flags/offline-features.test.ts | Updates offline-features test setup to simulate non-dotcom (GHES) instead of CCR. |
| src/feature-flags.ts | Removes CCR-specific offline feature-flag path in initFeatures. |
| src/feature-flags.test.ts | Removes the CCR-specific initFeatures test. |
| src/config-utils.ts | Switches “ignore generated files” gating from CCR to dynamic workflows; updates comment (needs follow-up). |
| src/actions-util.ts | Removes CCR detection and makes isDefaultSetup() equivalent to isDynamicWorkflow(). |
| src/actions-util.test.ts | Removes CCR test; leaves a now-misleading env var setup in the default-setup test (needs follow-up). |
| pr-checks/checks/ccr.yml | Removes the CCR pr-check definition. |
| .github/workflows/__ccr.yml | Removes generated CCR workflow (generated file; not reviewed). |
| .github/pull_request_template.md | Updates risk assessment checklist/options to remove CCR references and add “Development/testing only”. |
| lib/*.js | Generated output updates corresponding to src changes (not reviewed). |
| // If we are in a dynamic workflow or the corresponding FF is enabled, try to determine | ||
| // which files in the repository are marked as generated and add them to | ||
| // the `paths-ignore` configuration. | ||
| if ((await features.getValue(Feature.IgnoreGeneratedFiles)) && isCCR()) { | ||
| if ( | ||
| (await features.getValue(Feature.IgnoreGeneratedFiles)) && | ||
| isDynamicWorkflow() | ||
| ) { |
There was a problem hiding this comment.
The comment says "in a dynamic workflow or the corresponding FF is enabled", but the condition actually requires both: IgnoreGeneratedFiles must be enabled and isDynamicWorkflow() must be true. Update the comment to match the logic (or adjust the condition if the intent really is an OR).
There was a problem hiding this comment.
This is true, and I am pretty sure we had this on the PR that introduced it, and just like then I am not sure I want to re-run all CI for it 😆
| // the `paths-ignore` configuration. | ||
| if ((await features.getValue(Feature.IgnoreGeneratedFiles)) && isCCR()) { | ||
| if ( | ||
| (await features.getValue(Feature.IgnoreGeneratedFiles)) && |
There was a problem hiding this comment.
Change of behaviour, but should be safe since this FF isn't rolled out yet.
There was a problem hiding this comment.
Yeah, it's not rolled out. We also know since merging that feature that it may be more widely useful, so it makes sense to widen the condition here while we are at it.
2 participants
This is no longer needed.
Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Workflow types:
dynamicworkflows (Default Setup, CCR, ...).Products:
Environments:
github.comand/or GitHub Enterprise Cloud with Data Residency.How did/will you validate this change?
.test.tsfiles).pr-checks).If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist