GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Move bundle types and helpers out of setup-codeql.ts by henrymercer · Pull Request #4167 · github/codeql-action

Conversation

henrymercer commented Sep 24, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Move CodeQLDownloadSource and bundle asset naming into codeql-bundle.ts, and the per-language fallback warning into per-language-bundles.ts, so that other modules can use them without importing setup-codeql.ts. The moved naming function takes the platform as a parameter, so setup-codeql.ts now passes the same platform it uses to pick a per-language bundle. setup-codeql.ts still re-exports CodeQLDownloadSource. No behavior changes.

Risk assessment

Low risk: Refactor with no behavior changes, covered by existing unit tests.

Which use cases does this change impact?

Workflow types:

Advanced setup
Managed

Products:

Code Scanning
Code Quality
Other first-party - Analyses using the shared CodeQL setup path.

Environments:

Dotcom
GHES

How did/will you validate this change?

Unit tests - Existing tests for bundle naming, selection and per-language fallback. The bundle naming tests move to codeql-bundle.test.ts and pass the platform explicitly.
End-to-end tests - PR checks exercise the default and nightly download paths. These still need to run for this PR.

If something goes wrong after this change is released, what are the mitigation and rollback strategies?

Rollback - Revert or release a fix. The refactored code is not behind a feature flag.

How will you know if something goes wrong after this change is released?

Telemetry - Existing telemetry for tools downloads and initialization failures.

Are there any special considerations for merging or releasing this change?

No special considerations - This change can be merged at any time.

Merge / deployment checklist

Confirm this change is backwards compatible with existing workflows.
Consider adding a changelog entry for this change.
Confirm the readme and docs have been updated if necessary.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Move bundle types and helpers out of setup-codeql.ts

f42df5b

Co-authored-by: Copilot App <[email protected]>

github-actions Bot added the size/S Should be easy to review label

Sep 24, 2026

henrymercer requested a balanced review from Copilot

September 24, 2026 08:54

Copilot started reviewing on behalf of henrymercer

September 24, 2026 08:55

View session

Copilot AI reviewed

Sep 24, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

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.

Warning

Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Copilot review overview

🟢 Approval recommended

The refactor preserves existing behavior and public call patterns with relevant test coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Refactors bundle-related types and helpers into focused modules without changing behavior.

Changes:

Moves bundle naming and download-source definitions into codeql-bundle.ts.
Moves per-language fallback logging into per-language-bundles.ts.
Preserves existing setup-codeql.ts exports for compatibility.

File	Description
`src/setup-codeql.ts`	Delegates to relocated helpers while preserving exports.
`src/codeql-bundle.ts`	Defines bundle naming and download-source types.
`src/per-language-bundles.ts`	Adds the fallback-warning helper.
`lib/entry-points.js`	Generated output; excluded from review by policy.

Files excluded by content exclusion policy (1)

lib/entry-points.js

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Remove the getCodeQLBundleName wrapper from setup-codeql.ts

6aa6d55

Co-authored-by: Copilot App <[email protected]>

github-actions Bot added size/M Should be of average difficulty to review and removed size/S Should be easy to review labels

Sep 24, 2026

henrymercer marked this pull request as ready for review

September 24, 2026 09:36

henrymercer requested a review from a team as a code owner

September 24, 2026 09:36

mbg approved these changes

Sep 24, 2026

View reviewed changes

mbg left a comment

Copy link

Copy Markdown

Member

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.

LGTM. One minor comment, but I imagine that you'll have follow-up PRs to this one, so feel free to address it in one of those and merge this one as-is.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/per-language-bundles.ts

Comment on lines +135 to +136

		/** Explains why an eligible per-language bundle is being replaced by a combined bundle. */
		export function logPerLanguageBundleFallback(

mbg Sep 24, 2026

Copy link

Copy Markdown

Member

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.

Minor: Since this is just one possible reason why we might use a combined bundle rather than a per-language one, the "eligible" in the comment is doing a lot of heavy lifting and the function name doesn't make it clear at all. Consider

Suggested change

	/** Explains why an eligible per-language bundle is being replaced by a combined bundle. */
	export function logPerLanguageBundleFallback(
	/** Logs that we are using a combined bundle because no per-language bundle was found. */
	export function logNoPerLanguageBundle(

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

henrymercer Sep 24, 2026

Copy link

Copy Markdown

Contributor Author

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.

We'll only ever log this when we expected there to be a per-language bundle, hence the "fallback" in the function name, but the log message doesn't mention that one was expected. I'll update this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mbg reacted with thumbs up emoji

All reactions

👍 1 reaction

mbg Sep 24, 2026

Copy link

Copy Markdown

Member

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.

Thanks for the context regarding the circumstances of when logPerLanguageBundleFallback would be called at runtime, but that context is not necessarily obvious when looking at the source code here and from that angle I think my comment applies regardless?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

henrymercer added this pull request to the merge queue

Sep 24, 2026

Merged via the queue into main with commit 9c97080

Sep 24, 2026

234 checks passed

Uh oh!

There was an error while loading. Please reload this page.

henrymercer deleted the henrymercer/move-bundle-helpers branch

September 24, 2026 10:51

henrymercer mentioned this pull request

Sep 24, 2026

Select CodeQL bundles from a release #4172

Closed

This was referenced Oct 8, 2026

Merge main into releases/v4 #4214

Merged

Merge releases/v4 into releases/v3 #4216

Merged

Reviewers

mbg

mbg approved these changes

Copilot code review Copilot

Copilot review effort

Applies to this pull request for everyone.Learn more about Copilot code review.

Copilot left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

size/M

Should be of average difficulty to review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

henrymercer commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Risk assessment

Which use cases does this change impact?

How did/will you validate this change?

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

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

mbg left a comment

Choose a reason for hiding this comment

Uh oh!

mbg Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

henrymercer Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

mbg Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

henrymercer commented Sep 24, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.