-
-
Notifications
You must be signed in to change notification settings - Fork 7
Fixed signature HttpWorkerInterface #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 3.x
Are you sure you want to change the base?
Conversation
WalkthroughThe HttpWorkerInterface’s respond method signature now includes an optional bool parameter endOfStream (default true), expanding from three to four parameters. This modifies the public interface and call pattern for streaming responses while preserving prior behavior when the new parameter is omitted. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant S as Server/App
participant W as HttpWorkerInterface
C->>S: HTTP Request
S->>W: respond(status, body, headers, endOfStream)
alt endOfStream = true
Note right of W: Final chunk sent<br/>Connection closed
W-->>C: Send body and terminate stream
else endOfStream = false
Note right of W: Chunked/streaming<br/>More data expected
W-->>C: Send body chunk, keep stream open
S->>W: respond(..., endOfStream=true) for final chunk
W-->>C: Send final chunk and close
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)src/HttpWorkerInterface.php (2)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/HttpWorkerInterface.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/HttpWorkerInterface.php (2)
src/PSR7Worker.php (1)
respond(88-97)src/HttpWorker.php (1)
respond(86-99)
🔇 Additional comments (1)
src/HttpWorkerInterface.php (1)
33-33: Verified: all HttpWorkerInterface implementations include the$endOfStreamparameter and match the updated signature.
src/HttpWorkerInterface.php
Outdated
| * that header. | ||
| */ | ||
| public function respond(int $status, string|\Generator $body, array $headers = []): void; | ||
| public function respond(int $status, string|Generator $body, array $headers = [], bool $endOfStream = true): void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix formatting and update the docblock.
Two issues to address:
- There's a double space before
bool $endOfStream - The docblock (lines 21-32) doesn't document the new
$endOfStreamparameter
Apply this diff to fix the formatting:
- public function respond(int $status, string|Generator $body, array $headers = [], bool $endOfStream = true): void;
+ public function respond(int $status, string|Generator $body, array $headers = [], bool $endOfStream = true): void;And update the docblock to include:
* @param HeadersList|array<array-key, array<array-key, string>> $headers $headers An associative array of the
* message's headers. Each key MUST be a header name, and each value MUST be an array of strings for
* that header.
+ * @param bool $endOfStream Whether this response marks the end of the stream. Default is true.
*/
public function respond(int $status, string|Generator $body, array $headers = [], bool $endOfStream = true): void;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function respond(int $status, string|Generator $body, array $headers = [], bool $endOfStream = true): void; | |
| * @param HeadersList|array<array-key, array<array-key, string>> $headers $headers An associative array of the | |
| * message's headers. Each key MUST be a header name, and each value MUST be an array of strings for | |
| * @param bool $endOfStream Whether this response marks the end of the stream. Default is true. | |
| */ | |
| public function respond(int $status, string|Generator $body, array $headers = [], bool $endOfStream = true): void; |
🤖 Prompt for AI Agents
In src/HttpWorkerInterface.php around line 33, the method signature currently
has an extra double space before "bool $endOfStream" and the docblock above
(lines ~21-32) does not document the new $endOfStream parameter; remove the
extra space so the signature reads with a single space before the type, and
update the docblock to include a @param bool $endOfStream description (e.g.,
whether this response is the final stream chunk) and any default behavior,
keeping formatting consistent with existing @param entries.
roxblnfk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't add parameters into interface in minor version. It's breaking change because the interface isn't marked as internal
Fixed signature HttpWorkerInterface
Summary by CodeRabbit