Skip to content

Conversation

@sanjomo
Copy link
Member

@sanjomo sanjomo commented Nov 29, 2025

Description

Brief description of the changes in this PR.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactoring
  • Test improvements
  • Build/tooling changes

Related Issue

Closes #(issue number)

Changes Made

Testing

  • All existing tests pass
  • New tests added for new functionality
  • Tests pass locally with mvn test
  • Integration tests pass (if applicable)

Checklist

  • Code follows project coding standards
  • Self-review completed
  • Code is commented where necessary
  • Documentation updated (if needed)
  • Commit messages follow conventional format
  • No merge conflicts
  • All CI checks pass

Additional Notes

Any additional information, screenshots, or context that reviewers should know.

Summary by CodeRabbit

  • Documentation
    • Enhanced API documentation with comprehensive guidance on listening to all client-emitted events through available listener methods.
    • Added detailed Acknowledgement Rules section explaining acknowledgment request behavior, processing order, and edge case handling.
    • Included practical examples demonstrating event listeners, acknowledgment handling, and broadcasting messages in different scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 29, 2025

Walkthrough

Documentation added for listening to all client-emitted events via server.addOnAnyEventListener() and server.onAny() APIs, including acknowledgement rules and broadcasting message examples.

Changes

Cohort / File(s) Summary
Documentation updates
docs/api/index.md
Added sections for onAny event listener APIs, acknowledgement rules (ackRequest behavior, first ack wins), and broadcasting message context with usage examples

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • Straightforward documentation additions with no code logic changes
  • Single file modification focused on clarifying existing API behavior

Possibly related PRs

Suggested reviewers

  • NeatGuyCoding

Poem

🐰 Ears perked up with onAny delight,
All events flow in, both left and right,
Acknowledgements win in the very first race,
Documentation now guides us with grace!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description contains only the template structure with all checkboxes unchecked and no actual content filled in; critical sections like 'Brief description', 'Type of Change', and 'Changes Made' lack substantive information. Fill in the description with actual details: provide a brief summary, select the appropriate type of change (Documentation update), list specific changes made, and complete relevant checklist items.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: updating documentation to add addOnAnyEventListener functionality, which aligns with the raw summary showing documentation additions for this API.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sanjomo-patch-1

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
docs/api/index.md (1)

58-63: Document both API variants with examples.

The documentation introduces two equivalent APIs (addOnAnyEventListener and onAny) but only the example uses addOnAnyEventListener. Adding a brief example or note clarifying that both are equivalent would improve clarity for users deciding which to use.

Consider adding a note like:

Both APIs are equivalent; choose based on your coding style preference:
- `server.addOnAnyEventListener(...)` - Method chaining style
- `server.onAny(...)` - Shorter alias

Or provide a brief second example:

// Alternative syntax using onAny alias
server.onAny((client, event, args, ackRequest) -> {
    // Handle any event
});
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ca5a904 and 6ca7b89.

📒 Files selected for processing (1)
  • docs/api/index.md (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build (17) / build
  • GitHub Check: build (21) / build
  • GitHub Check: build (25) / build
🔇 Additional comments (2)
docs/api/index.md (2)

66-83: Code example is clear and practical.

The example demonstrates proper usage of the catch-all listener: accessing client metadata, iterating arguments, and handling acknowledgments. The null-safety check with isAckRequested() before sending ack data is a good defensive pattern to highlight.


84-88: Acknowledgement rules are well-documented.

The three bullet points clearly explain the ack semantics: when acks are requested, the "first-wins" behavior, and what happens to subsequent ack attempts. This is important context that prevents common mistakes.

@NeatGuyCoding
Copy link
Collaborator

LGTM

@NeatGuyCoding NeatGuyCoding merged commit c4aba4e into main Nov 30, 2025
8 checks passed
@sanjomo sanjomo deleted the sanjomo-patch-1 branch December 14, 2025 09:11
@coderabbitai coderabbitai bot mentioned this pull request Jan 3, 2026
18 tasks
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