Skip to content

chore: clean up legacy leftovers, dead code and deprecated CI actions - #98

Open
moezdil wants to merge 3 commits into
lfedgeai:mainfrom
moezdil:chore/remove-stale-legacy-leftovers
Open

moezdil wants to merge 3 commits into
lfedgeai:mainfrom
moezdil:chore/remove-stale-legacy-leftovers

Conversation

@moezdil

@moezdil moezdil commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

removes dead code, unused deps, stale tests/docs and old ci bits, noticed a few more deprecations around the repo too but kept this one behavior-neutral.

@moezdil moezdil changed the title chore: remove stale legacy comments and commented-out CI steps chore: clean up legacy leftovers and deprecated CI actions Oct 25, 2026
@moezdil
moezdil force-pushed the chore/remove-stale-legacy-leftovers branch 3 times, most recently from 4392a28 to ce5e7dd Compare October 25, 2026 18:03
@moezdil moezdil changed the title chore: clean up legacy leftovers and deprecated CI actions chore: clean up legacy leftovers, dead code and deprecated CI actions Oct 25, 2026
@moezdil
moezdil force-pushed the chore/remove-stale-legacy-leftovers branch from ce5e7dd to 17288a0 Compare October 25, 2026 18:18
Remove long-unreferenced Rust modules, functions, macros and test helpers, and unused crate and npm dependencies. Fix ui-tests asserting a testid removed in lfedgeai#94, feature-gated examples/tests that no longer compiled with --all-features, and docs pointing at removed routes, files and env vars. Also drop Go/Python-era CI leftovers and bump actions on the deprecated Node 16 runtime.

Signed-off-by: mesutoezdil <[email protected]>
@moezdil
moezdil force-pushed the chore/remove-stale-legacy-leftovers branch from 17288a0 to 7e583af Compare October 25, 2026 18:24

@rogueprocess13 rogueprocess13 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you for this cleanup. I searched the rest of the repository for each removed symbol, macro, and dependency. The search covered the code behind the sled and rocksdb features, the standalone crates (SDK, samples, spear-quickstart), the Makefile and Dockerfiles, and the imports in web-admin and web-console. Nothing still uses the removed items. The change to action-gh-release@v2 with name: also fixes the release title, because release_name was never a valid input for that action.

Docs to fix before merge

Two docs still refer to code that this PR removes. docs/backend-adapter/implementation-plan-en.md (lines 123 and 154) and its -zh version point to src/spearlet/execution/ai/media_ref.rs. docs/config-service-refactoring-en.md and its -zh version describe AppConfig::load() and figment::Error as the way to load the configuration. This PR removes that trait and the figment dependency.

Optional changes

These items are not required for merge:

  • connect_sms_channel_once and connect_sms_channel_retry (src/spearlet/sms_connector.rs:52 and :61) have no callers now. Their only callers were the connect_sms_channel_status_* wrappers that this PR removes.
  • TaskInstance.listening_address (src/spearlet/execution/instance.rs:214) is written but never read now. The process runtime still calls set_listening_address, but get_listening_address was the only reader.
  • The task-node assertions in ui-tests/tests/task_create_auto_schedule.spec.ts and executable_select_unit.spec.ts were correct to remove, because #94 removed the node picker. To keep the intent of the auto-schedule test, you can add await expect(page.getByTestId('task-node')).toHaveCount(0).
  • The previous cleanup has an entry with a verification section in docs/DEAD_CODE_CLEANUP-{en,zh}.md. A short entry for this PR will keep that record complete.

Build and test results

No CI ran on this PR, because core-build.yaml runs only on push. My review was static: I searched and read the code, but I did not build it. Please post the output of cargo check --all-features --all-targets and make test. Use --all-features, because CI builds only the default features, and some of the removed code compiled only with sled or rocksdb.

Separate from this PR: is_scaling_cooldown_over also had no callers on main, so nothing enforces ScalingConfig.cooldown_period_ms. This can be a separate issue.

@moezdil

moezdil commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for this cleanup. I searched the rest of the repository for each removed symbol, macro, and dependency. The search covered the code behind the sled and rocksdb features, the standalone crates (SDK, samples, spear-quickstart), the Makefile and Dockerfiles, and the imports in web-admin and web-console. Nothing still uses the removed items. The change to action-gh-release@v2 with name: also fixes the release title, because release_name was never a valid input for that action.

Docs to fix before merge

Two docs still refer to code that this PR removes. docs/backend-adapter/implementation-plan-en.md (lines 123 and 154) and its -zh version point to src/spearlet/execution/ai/media_ref.rs. docs/config-service-refactoring-en.md and its -zh version describe AppConfig::load() and figment::Error as the way to load the configuration. This PR removes that trait and the figment dependency.

Optional changes

These items are not required for merge:

  • connect_sms_channel_once and connect_sms_channel_retry (src/spearlet/sms_connector.rs:52 and :61) have no callers now. Their only callers were the connect_sms_channel_status_* wrappers that this PR removes.
  • TaskInstance.listening_address (src/spearlet/execution/instance.rs:214) is written but never read now. The process runtime still calls set_listening_address, but get_listening_address was the only reader.
  • The task-node assertions in ui-tests/tests/task_create_auto_schedule.spec.ts and executable_select_unit.spec.ts were correct to remove, because refactor: implement unified AI backend control plane #94 removed the node picker. To keep the intent of the auto-schedule test, you can add await expect(page.getByTestId('task-node')).toHaveCount(0).
  • The previous cleanup has an entry with a verification section in docs/DEAD_CODE_CLEANUP-{en,zh}.md. A short entry for this PR will keep that record complete.

Build and test results

No CI ran on this PR, because core-build.yaml runs only on push. My review was static: I searched and read the code, but I did not build it. Please post the output of cargo check --all-features --all-targets and make test. Use --all-features, because CI builds only the default features, and some of the removed code compiled only with sled or rocksdb.

Separate from this PR: is_scaling_cooldown_over also had no callers on main, so nothing enforces ScalingConfig.cooldown_period_ms. This can be a separate issue.

thx, PTAL!

@wilsonwang371

Copy link
Copy Markdown
Collaborator

Hi Mesut @moezdil , how do you using SPEAR in your environment? what do you use it for?

@moezdil

moezdil commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Hi Mesut @moezdil , how do you using SPEAR in your environment? what do you use it for?

Hi, I don't use it in my environment.

@wilsonwang371

Copy link
Copy Markdown
Collaborator

In this case, what is the purpose of this patch? Is it really affect any usecase?
@moezdil

@moezdil

moezdil commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor Author

In this case, what is the purpose of this patch? Is it really affect any usecase?

@moezdil

What you mean with usecase? It is clean up. As you see in Titel and or description, the repo is not uptodate. For next step it would be better to merge

@wilsonwang371

Copy link
Copy Markdown
Collaborator

I have a couple of questions before we merge:

This removes a few public methods, like TaskInstance::generate_request_id and InstanceScheduler::with_config. Even if nothing in this repo calls them, could downstream users depend on them? Are we okay with that API break?
I don’t see any CI results yet. Could you run cargo check --all-features --all-targets and make test and share the results?
One small docs thing: the cleanup log says “2026-06,” but this PR is from October. Could you update the date and add the actual verification results?
Once we’ve cleared these up, I think it’ll be much easier to confirm this is a safe cleanup.

@moezdil

moezdil commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor Author

hm good questions

i // ran cargo check --all-features --all-targets: clean. make test: 630 passed, 0 failed. cargo test --all-features: 700 passed, 0 failed. Results are also in the cleanup log.

ii// fixed the cleanup log date to 2026-10 and replaced "see PR" with the actual results

iiii//
and last point is little bit tricky

what i see:

neither method had any caller before removal.
and spear-next isnt on crates.io and GitHub code search shows no external dependents.
do you know any consumers, i cannot knwo it, but in any case, happy to restore them if you know of one.
these are in any case dead codes.. and old version.

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.

3 participants