-
Notifications
You must be signed in to change notification settings - Fork 1.9k
v.2.6.0-2 release proposal #1041
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
@@ -51,6 +49,7 @@ This will display: | |||
mjr:~/work/node_redis (master)$ | |||
|
|||
Note that the API is entirely asynchronous. To get data back from the server, you'll need to use a callback. | |||
From v.2.6 on the API supports camelCase and snack_case and all options / variables / events etc. can be used either way. |
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.
Perhaps mention that it also supports UPPERCASE? ..but that camelCase is advised because of Nodejs conventions
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.
Good hint. But all-caps is not supported for events, variables and options at the moment. Implementing this too would be easy, but I'm wondering if it might be worth thinking about removing support for it in v.3. It was implemented in the very first commit of node_redis but should it also be used that way? I'd disagree.
It is actually documented somewhere else but documenting it further might encourage people to use it, so removing it afterwards would be more work for users to switch.
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.
Sorry for the confusion, I totally agree that options, events etc etc shouldn't be passed as uppercase. I was only referring to the method names / redis commands. Probably should have read the whole readme before leaving this 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.
Right now I'm actually thinking about removing the documentation about the UPPER_CASE commands support as this is similar to snack_case not typical in Node.js landscape.
In general conventions as I know them this is actually even more confusing as uppercase mostly stands for constants and this is clearly not the case here.
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.
👍
d7857bd
to
2decc90
Compare
@BridgeAR When this PR will be merged ? I'm waiting for this version, because you fixed problem with AUTH. |
Fixes missing `EXEC_BATCH` on multi
If the command_queue and the offline_queue holds commands, the offline_queue should be choosen instead of the command_queue.
Fixes #1028
Fix quit possibly resulting in reconnections
Arguments are now passed to an command error in case they exist An error is only emitted if that very same error is not already handled in a callback
The total size is kept in the queue but this does not have to be reset each time
…om now on This removes the optional-dev-dependency as this is not needed from now on anymore
… in the tls options
Add command sanity check
a6acfdd
to
3084deb
Compare
Added individual error classes Don't silently fail for commands without callback from now on General polishing (e.g. better error messages) Fix typos
Pull Request check-list
npm test
pass with this change (including linting)?Description of change
This is a release proposal for the hopefully last v.2.6 pre-release.
There are quite a few nice improvements and even further stability fixes included.
_A code review is highly welcome!_ Ping @NodeRedis/contributors @dirkbonhomme
Fixes #1035 #1028
Maybe fixes #1037