feat: add promise-based service calls #1311
Conversation
mahmoud-ghalayini
force-pushed
the
feature/promise-based-service-calls-1310
branch
from
59cf513 to
ae08f65
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds promise-based service calls to the rclnodejs library through a new sendRequestAsync() method, providing modern async/await patterns as an alternative to the existing callback-based sendRequest() method.
Key changes:
- Added
sendRequestAsync()method to the Client class with timeout and cancellation support - Comprehensive test coverage for the new async functionality
- Documentation and examples demonstrating usage patterns
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/client.js | Implements sendRequestAsync() method with Promise-based API, timeout handling, and AbortSignal support; also fixes typos in existing documentation |
| test/test-async-client.js | Comprehensive test suite covering all aspects of async client functionality including timeouts, cancellation, concurrent requests, and error handling |
| example/services/client/async-client-example.js | Example demonstrating basic usage of the new async client API |
| example/services/README.md | Documentation updates explaining the new async API, usage patterns, and differences from callback-based approach |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
@mahmoudalghalayini thanks for submitting the PR, just a comment about the parameter used for the added function. BTW, please feel free to add your name into CONTRIBUTORS
mahmoud-ghalayini
force-pushed
the
feature/promise-based-service-calls-1310
branch
from
ae08f65 to
b957ff4
Compare
mahmoud-ghalayini
force-pushed
the
feature/promise-based-service-calls-1310
branch
from
b957ff4 to
6d1e681
Compare
|
@mahmoud-ghalayini I have approved, please add some description when merging, thanks! |
Description is added, but I don't have merge right 😄 |
@mahmoud-ghalayini I have added you as collaborator, please check your email inbox to see any invitation received. |
4 participants
#1310