Skip to content

Conversation

mbg
Copy link
Member

@mbg mbg commented Oct 10, 2025

We merged support for this in #3175. It might be useful to be able to disable this functionality, so this PR adds a FF check for it.

Risk assessment

For internal use only. Please select the risk level of this change:

  • Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Which use cases does this change impact?

  • Advanced setup - Impacts users who have custom workflows.
  • Default setup - Impacts users who use default setup.
  • Code Scanning - Impacts Code Scanning (i.e. analysis-kinds: code-scanning).
  • Code Quality - Impacts Code Quality (i.e. analysis-kinds: code-quality).
  • Third-party analyses - Impacts third-party analyses (i.e. upload-sarif).

How did/will you validate this change?

  • Unit tests - I am depending on unit test coverage (i.e. tests in .test.ts files).
  • End-to-end tests - I am depending on PR checks (i.e. tests in pr-checks).

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

  • Feature flags - All new or changed code paths can be fully disabled with corresponding feature flags.

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

  • Telemetry - I rely on existing telemetry or have made changes to the telemetry.
    • Alerts - New or existing monitors will trip if something goes wrong with this change.

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.

mbg added 2 commits October 10, 2025 13:27
Mainly to allow us to disable it, if needed.
@mbg mbg requested a review from henrymercer October 10, 2025 12:51
@mbg mbg requested a review from a team as a code owner October 10, 2025 12:51
@Copilot Copilot AI review requested due to automatic review settings October 10, 2025 12:51
Copy link
Contributor

@henrymercer henrymercer left a comment

Choose a reason for hiding this comment

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

Consider adding a test too. Otherwise just one small thing to fix and one general comment.

UseRepositoryProperties = "use_repository_properties",
QaTelemetryEnabled = "qa_telemetry_enabled",
ResolveSupportedLanguagesUsingCli = "resolve_supported_languages_using_cli",
UseRepositoryProperties = "use_repository_properties",
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: Is there a lint rule for alphabetical enums we can enable?

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@mbg mbg force-pushed the mbg/ff/tools-toolcache branch from ec1db76 to dc2ced8 Compare October 10, 2025 13:40
@mbg mbg requested a review from henrymercer October 10, 2025 13:40
@mbg mbg requested a review from henrymercer October 10, 2025 14:07
@mbg mbg merged commit 4e90a42 into main Oct 10, 2025
243 checks passed
@mbg mbg deleted the mbg/ff/tools-toolcache branch October 10, 2025 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants