Remove TRAP caching feature flag - #1533
cklin merged 1 commit into
Conversation
Sorry, something went wrong.
angelapwen
left a comment
There was a problem hiding this comment.
Looks good to me, reran the PR check timeout.
Sorry, something went wrong.
There was a problem hiding this comment.
Typically, removing the feature flag check in the Action would enable the feature on GHES, since features are off by default in GHES. However since getTrapCachingEnabled() returns false for self-hosted runners, TRAP caching is effectively disabled for GHES. So I agree this PR doesn't change behaviour in practice — the only change will be to enable TRAP caching for PRs without permission to access the API, such as Dependabot PRs.
Sorry, something went wrong.
Thanks for pointing that out! For future reference, what do we usually do for GHES in feature flag removals? If we want the feature to remain disabled in GHES, we could use util.getGitHubVersion() and specifically exclude GHES. What about features that we do want to enable on GHES? |
Sorry, something went wrong.
|
Most of the time, we want to enable the feature on GHES, which we do by dropping the feature flag check, and making sure there aren't any code paths to specifically disable GHES, like you mention. |
Sorry, something went wrong.
This PR makes the CodeQL action assume that the trap_caching_enabled is true without actually checking it. The feature flag has been set to true globally for months, so this change should have no practical effect.
Merge / deployment checklist