feat: broadcastWithOptions ACTR-136 #939
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
abcxff
force-pushed
the
04-20-feat_ai-agent_example
branch
from
090fe2a to
317f869
Compare
abcxff
force-pushed
the
04-22-feat_broadcastwithoptions_actr-136
branch
from
b14834e to
7bf1236
Compare
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| /** | ||
| * The connection IDs to be excluded from the broadcast. | ||
| */ | ||
| exclude?: string[]; |
There was a problem hiding this comment.
The type for exclude should be ConnId[] rather than string[] to maintain consistency with the BroadcastOptions interface defined in action.ts. This ensures type safety across the codebase and prevents potential issues when passing connection IDs between these interfaces.
| exclude?: string[]; | |
| exclude?: ConnId[]; |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| const exclude = opts.exclude ?? []; | ||
|
|
||
| if (opts.excludeSelf) { | ||
| exclude.push(this.conn.id); | ||
| } |
There was a problem hiding this comment.
Potential Issue: This code mutates the exclude array that was passed in through opts. If the caller reuses this array elsewhere, they might encounter unexpected behavior. Consider creating a copy of the array instead:
const exclude = [...(opts.exclude ?? [])];This ensures the original array remains unchanged while still allowing the function to add the current connection ID when excludeSelf is true.
| const exclude = opts.exclude ?? []; | |
| if (opts.excludeSelf) { | |
| exclude.push(this.conn.id); | |
| } | |
| const exclude = [...(opts.exclude ?? [])]; | |
| if (opts.excludeSelf) { | |
| exclude.push(this.conn.id); | |
| } |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
No description provided.