Skip to content

refactor(tools): Improve Actor loader names - #1181

Open
Jonesxq wants to merge 1 commit into
apify:masterfrom
Jonesxq:refactor/improve-actor-loader-names
Open

refactor(tools): Improve Actor loader names#1181
Jonesxq wants to merge 1 commit into
apify:masterfrom
Jonesxq:refactor/improve-actor-loader-names

Conversation

@Jonesxq

@Jonesxq Jonesxq commented Jul 30, 2026

Copy link
Copy Markdown

Why

Closes #758.

The existing names blur two abstraction levels and use get for a helper that fetches data from the Apify API.

What changed

  • Renamed getActorsAsTools() to fetchActorsAsTools().
  • Renamed getActors() to getActorsFromInput().
  • Updated their exports, consumers, tests, mocks, and documentation.

Notes for reviewers (human-written)

Proof it works

  • An exact-symbol repository search finds no remaining references to either previous name.
  • corepack pnpm run format
  • corepack pnpm run type-check
  • corepack pnpm run lint (0 warnings and errors)
  • corepack pnpm run check:agents (7 docs checked)
  • corepack pnpm run test:unit (1,246 passed, 1 skipped)

Integration tests were not run because repository instructions reserve them for humans with an APIFY_TOKEN.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@jirispilka
jirispilka self-requested a review July 31, 2026 21:19

@jirispilka jirispilka left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for this, the rename itself looks clean.

One heads-up first: #758 was stale, and its good first issue label made it look like an open invitation, that's on us, and we'll clean it up. We're tightening the process in #1192, so for future changes please ping the issue and wait for a maintainer before coding. Nothing you did wrong here.

A few things to fix while you're in there:

getActorsFromInput → fetchActorsFromInput. The premise of the PR is that get is wrong for a helper that calls the Apify API — but this one is async, awaits fetchActorsAsTools, and its own JSDoc opens with "Fetch Actor tool entries…". CONTRIBUTING.md:91-92 is the rule to follow here, not #758's wording: get for cheap local lookups, fetch for async external calls. fetchActorsFromInput keeps the distinction #758 wanted (FromInput vs AsTools) with both verbs telling the truth. The issue got this wrong, not you.

src/mcp/server.ts:473 — the reflowed comment line is now noticeably longer than its neighbours; a quick re-wrap keeps the block tidy.
Optional, same rule as (1): getMCPServersAsTools also hits the network and kept get. If you want to finish the job, fetchMCPServersAsTools fits. (getNormalActorsAsTools is fine as-is — it's a pure transform over already-fetched data.)

Last thing: "Notes for reviewers (human-written)" is empty, and the description doesn't mention AI assistance. If an agent helped, name the tool and what it did, and add a couple of sentences in your own words — reviewers need to know which parts a human has read.

And we typically require a proof that the server works, typically using mcpc

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.

Improve name for getActors and getActorsAsTools

3 participants