-
-
Notifications
You must be signed in to change notification settings - Fork 8
Add promise support with backward-compatible callback detection #16
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
Conversation
Co-authored-by: fvdm <93784+fvdm@users.noreply.github.com>
Co-authored-by: fvdm <93784+fvdm@users.noreply.github.com>
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.
Pull request overview
This pull request adds promise/async-await support to the vnstat-dumpdb library while maintaining full backward compatibility with the existing callback-based API. The implementation uses a promisify() wrapper function that detects whether a callback is provided as the last argument—if so, it uses the traditional callback pattern; otherwise, it returns a Promise.
Key Changes:
- Added a
promisify()helper function that wraps callback-based functions to support both callback and promise patterns - Wrapped
getStats()andgetConfig()exports with the promisify function without modifying their internal implementation - Added 6 comprehensive promise-based tests covering success and error scenarios with async/await patterns
- Updated README documentation with promise/async-await usage examples and API signature changes
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| vnstat-dumpdb.js | Added promisify() wrapper function and applied it to getStats and getConfig exports to enable promise support |
| test.js | Added 6 promise-based tests covering .getConfig(), .getStats() with various parameters, and error handling scenarios |
| README.md | Updated documentation with promise usage examples, updated method signatures to show optional callbacks, and added promise-style code examples throughout |
Comments suppressed due to low confidence (1)
vnstat-dumpdb.js:78
- The JSDoc comment for the
getConfigfunction should be updated to reflect that the callback parameter is now optional. Since the function is wrapped withpromisify(), it can be called without a callback and will return a Promise. The documentation should indicate this with@param {function} [callback]instead of@param {function} callback.
/**
* Get vnStat config
*
* @param {function} callback `(err, data)`
* @returns {void}
* @callback callback
*/
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vnstat-dumpdb.js
Outdated
| * Helper to promisify a function that uses callbacks | ||
| * | ||
| * @param {function} fn The function to promisify | ||
| * @returns {function} Wrapped function that returns a Promise if no callback |
Copilot
AI
Jan 5, 2026
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.
The JSDoc for the promisify function should specify that the returned wrapped function can return either void (when a callback is provided) or a Promise (when no callback is provided). Consider updating the @returns tag to be more explicit, such as @returns {function} Wrapped function that returns void if callback provided, or Promise if not.
| * @returns {function} Wrapped function that returns a Promise if no callback | |
| * @returns {function} Wrapped function that returns void if callback provided, or Promise if not |
|
@copilot Update JSDoc to reflect the now optional callback function and return {void|Promise} |
Co-authored-by: fvdm <93784+fvdm@users.noreply.github.com>
|
@copilot Bump and tag the version in |
Co-authored-by: fvdm <93784+fvdm@users.noreply.github.com>
Plan: Add Promise Support with Backward Compatibility
getConfig()to support both callbacks and promisesgetStats()to support both callbacks and promisesSummary
Successfully added asynchronous promise support to vnstat-dumpdb while maintaining 100% backward compatibility.
Version 2.1.0 Release:
This minor version bump reflects the new promise/async-await feature addition while maintaining full backward compatibility with existing callback-based code.
Key Features:
promisify()- Returns void if callback provided, or Promise if notgetConfig([callback])- Callback now optional, returns{void|Promise}getStats([iface], [callback])- Both parameters optional, returns{void|Promise}Test Results:
Original prompt
This pull request was created from Copilot chat.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.