Make FFs available in start-proxy action - #3438
Conversation
Sorry, something went wrong.
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.
Sorry, something went wrong.
| features = new Features( | ||
| gitHubVersion, | ||
| repositoryNwo, | ||
| actionsUtil.getTemporaryDirectory(), | ||
| logger, | ||
| ); |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
Copilot Autofix
AI 7 months 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 features with a single new 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.
Autofix patch
cat << 'EOF' | git apply
diff --git a/src/start-proxy-action.ts b/src/start-proxy-action.ts
--- a/src/start-proxy-action.ts
+++ b/src/start-proxy-action.ts
@@ -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(),
EOF
| @@ -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 Features object available in the start-proxy action. 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:
Products:
Environments:
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