Skip to content

Refactor Execution::Next runner flow - #5726

Open
ydah wants to merge 1 commit into
rmosolgo:masterfrom
ydah:refactor-execution-next-runner
Open

Refactor Execution::Next runner flow#5726
ydah wants to merge 1 commit into
rmosolgo:masterfrom
ydah:refactor-execution-next-runner

Conversation

@ydah

@ydah ydah commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

GraphQL::Execution::Runner#execute and #begin_execute currently handle multiplex analysis, query setup, step execution, result finalization, root authorization, operation directives, and root-type dispatch inline.

This PR reorganizes those responsibilities into private methods:

  • Separates multiplex analysis, query setup, step execution, and result finalization.
  • Separates object, abstract, list, and scalar root execution.
  • Extracts root authorization, operation directive handling, and mutation setup.
  • Consolidates repeated root SelectionsStep construction.
  • Keeps isolated mutation step groups on the runner so setup methods can enqueue work.

The existing result ordering, tracing boundaries, directive finalization, mutation isolation, and stack-error handling are preserved.

@rmosolgo

rmosolgo commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Could you please isolate the bug fix from the code reorganization? The bug fix is an easy include, but I doubt I want to include the code reorganization. Whenever possible, I prefer a single method because I can read it from top-to-bottom and know what's going on. When the code is divided into methods, then the reader (me?) has to jump around to find out what's really going on.

From a maintenance perspective, I prefer keeping API surface area as small as possible: private instead of public whenever possible, inline instead of extracted whenever possible, local variable instead of instance variable whenever possible, and so on.

@ydah
ydah force-pushed the refactor-execution-next-runner branch from 22971c2 to 60f2067 Compare September 2, 2026 12:17
@ydah

ydah commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

@rmosolgo Thank you, that makes sense.
I've separated the changes and force-pushed this PR so it now contains only the code reorganization.

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.

2 participants