Conversation
3524024 to
d2901f5
Compare
henrymercer
left a comment
There was a problem hiding this comment.
In general, I'd prefer to avoid adding operations that take time and are unused, but in this case we're very likely to use the feature flags in another Action, meaning we'd need to make the API call anyway. So LGTM.
| features = new Features( | ||
| gitHubVersion, | ||
| repositoryNwo, | ||
| actionsUtil.getTemporaryDirectory(), | ||
| logger, | ||
| ); |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 15 hours ago
To fix the problem, we should remove the unused local variable and its assignment while preserving any intentional side effects from constructing Features. Instead of declaring let features and assigning to it, we can (a) remove the features declaration, and (b) replace features = new Features(...) with a bare new Features(...) call. This keeps the intended “initialise FFs” behavior but no longer suggests that a features value is used later.
Concretely in src/start-proxy-action.ts:
- Delete the
let features: Features | undefined;declaration at line 98. - Replace lines 113–119 that currently disable ESLint and assign to
featureswith a singlenew Features(...)expression, keeping the parameters unchanged and removing the ESLint suppression because there will no longer be an unused identifier.
No new imports, methods, or other definitions are required.
| @@ -95,7 +95,6 @@ | ||
| // possible, and only use safe functions outside. | ||
|
|
||
| const logger = getActionsLogger(); | ||
| let features: Features | undefined; | ||
| let language: KnownLanguage | undefined; | ||
|
|
||
| try { | ||
| @@ -110,8 +109,7 @@ | ||
| // Initialise FFs, but only load them from disk if they are already available. | ||
| const repositoryNwo = getRepositoryNwo(); | ||
| const gitHubVersion = await getGitHubVersion(); | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| features = new Features( | ||
| new Features( | ||
| gitHubVersion, | ||
| repositoryNwo, | ||
| actionsUtil.getTemporaryDirectory(), |
(Based on #3464 which should be merged first.)
This PR makes a
Featuresobject available in thestart-proxyaction. While we don't make any use of this yet, we plan to in later changes.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:
analysis-kinds: code-scanning.analysis-kinds: code-quality.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