A.#13a: delete vestigial app-instances + app-variables legacy configs#5345
Merged
norman-abramovitz merged 4 commits intoMay 18, 2026
Conversation
The legacy CfAppVariablesDataSource hosted the ListAppEnvVar interface as a side-export — five consumers (signal-config, variables-tab, table-cell-edit-variable + its spec, app-variables actions, application-env-var action-builders) all pulled it from the data-source file. Lifting the type to a tiny cf-app-variables.types.ts unblocks deleting the now-vestigial data-source + legacy list-config in the next slice. Pure import-path shuffle; ListAppEnvVar shape is unchanged.
The cf-app-instances-config.service.ts (now deleted in the prior commit) hosted a 7-line createAppInstancesMetricAction helper that app.effects.ts pulls in to fire the firehose CPU metric query on UpdateExistingApplication. Lifting the helper to its own cf-app-instances-metrics-action.ts module kept that effect path intact across the legacy-config deletion. Pure import-path shuffle; helper shape is unchanged.
The signal-configs + instances-tab + app-variables.effects carried multi-paragraph context comments naming the now-deleted legacy classes (CfAppInstancesConfigService, CfAppVariablesListConfigService, cf-app-variables-data-source.ts:56/66, …). Those references were historical migration notes — useful at extraction time, rotted once the legacy files left the tree. Comments rewritten to describe current behavior + WHY only — no mention of "legacy / mirrors / replaces / was-in-X.ts". Migration history lives in git log; the source files should read as if signal was always the implementation.
|
norman-abramovitz
approved these changes
May 18, 2026
Contributor
norman-abramovitz
left a comment
There was a problem hiding this comment.
Removing dead code
ffb393f
into
cloudfoundry:develop
11 of 12 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Spike of A.#13 (apps-wall data-source bundle) found that 2 of the 5 candidate data sources are fully vestigial — both signal-native and legacy paths are wired, but only the signal path is reachable from any routed UI. This PR removes the dead legacy path:
cf-app-instances-data-source.ts— never instantiated outside its sibling config filecf-app-instances-config.service.ts+ spec — referenced only by comments in the signal-config / instances-tabcf-app-variables-data-source.ts— never instantiated outside its sibling config filecf-app-variables-list-config.service.ts+ spec — referenced only by comments in the signal-configThe remaining 3 candidates from the original A.#13 plan stay deferred:
cf-apps-data-source.ts— runtime-vestigial but exports apaginationKey+includeRelationsconsumed by create-application / deploy-application (themselves still ngrx; goes with A.Creating event bus service #15)cf-space-apps-data-source.service.ts— still drives the livecloud-foundry-space-appspage via the legacyListComponent/ListConfigpattern (real migration, A.#13b)cf-space-routes-data-source.ts— same shape for the space-routes page (A.#13b)Commit slices
ListAppEnvVarto its own types module — 5 live consumers (signal-config, variables-tab, table-cell-edit-variable + spec, app-variables actions, application-env-var action-builders) used to pull this 3-field type from the data-source file. Lifting tocf-app-variables.types.tsunblocks deletion. Also drops the 6 vestigial files.createAppInstancesMetricActionhelper —app.effects.tsimports this 7-line helper from the now-deleted instances config file. Moved to its owncf-app-instances-metrics-action.tsmodule.Why this matters
This eliminates 2 of B.2's 26 live blockers for the entity-relations module deletion. Three more PRs (A.#13b + A.#15 + A.#5/8/9 followups) push the rest down before B.2 can ship.
Test plan
vitest run app-instance app-variables variables-tab— 61 tests passvitest run app-instances-signal app-variables-signal variables-tab(post-comment-scrub) — 29 tests passmake check gate— left for PR CIMerge as Create a merge commit to preserve the 4-slice identity.