Skip to content

Decorate Multi objects returned by client #15

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 30, 2019
Merged

Decorate Multi objects returned by client #15

merged 2 commits into from
Oct 30, 2019

Conversation

@jd20
Copy link
Contributor

@jd20 jd20 commented Jan 21, 2019

No description provided.

@jd20 jd20 mentioned this pull request Jan 21, 2019
Closed
@coveralls
Copy link

coveralls commented Jan 21, 2019 •
edited
Loading

Pull Request Test Coverage Report for Build 40

  • 17 of 17 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%
Totals Coverage Status
Change from base Build 35: 0.0%
Covered Lines: 35
Relevant Lines: 35
💛 - Coveralls

@moaxaca
Copy link
Owner

moaxaca commented Jan 21, 2019

I will test this later this week.

@jd20
Copy link
Contributor Author

jd20 commented Feb 6, 2019

One issue I've noticed, if you have code that is calling multi() a lot, the performance overhead of decorating the Multi object every time can be significant. I'm thinking we might want a smarter way to do the decoration only once.

@moaxaca
Copy link
Owner

moaxaca commented Feb 6, 2019

@jd20 Yea I've been considering overhauling parts of the library. Making more of a proxy class over attempting to decorate it.

@moaxaca
Copy link
Owner

moaxaca commented Feb 6, 2019

Could you clean up the linting errors caught by CI?

@jd20
Copy link
Contributor Author

jd20 commented Feb 6, 2019

@moaxaca Yes, a proxy class should improve performance greatly. I pushed a commit for the linter errors.

@moaxaca
Copy link
Owner

moaxaca commented Feb 6, 2019

I will look into a proxy implementation with an identical interface.

Thanks for contributing.

ThisIsMissEm
ThisIsMissEm reviewed Aug 13, 2019
View reviewed changes
src/index.js
// this is the set of Multi commands to promisify
const multiCommandSet = new Set(['exec', 'exec_atomic']);

const promisify = function (object, method) {
Copy link

Choose a reason for hiding this comment

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

It should be possible to use utils.promisify now too: https://nodejs.org/dist/latest-v10.x/docs/api/util.html#util_util_promisify_original

(It's since v8 of node)

@moaxaca moaxaca merged commit 71d4a87 into moaxaca:develop Oct 30, 2019
moaxaca added a commit that referenced this pull request May 25, 2021
@moaxaca @jd20
@DreadPirateSergey @jakesylvestre @dependabot @cpeddecord
Release 2.0.0 (#32)
513c965 
* Decorate Multi objects returned by client (#15)

* Decorate Multi objects returned by client
* Fix linter warnings

* Remove mutating of original redisClient object. (#37)

* Add link to node redis library

* Bump stringstream from 0.0.5 to 0.0.6 (#27)

Bumps [stringstream](https://github.com/mhart/StringStream) from 0.0.5 to 0.0.6.
- [Release notes](https://github.com/mhart/StringStream/releases)
- [Commits](mhart/StringStream@v0.0.5...v0.0.6)

Signed-off-by: dependabot[bot] <support@github.com>

* Remove mutating of original redisClient object

Co-authored-by: Jake <jakesyl@gmail.com>
Co-authored-by: Matthew Oaxaca <mattoaxaca@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* upgrading redis to 3.0.0 (#36)

* Add link to node redis library

* Bump stringstream from 0.0.5 to 0.0.6 (#27)

Bumps [stringstream](https://github.com/mhart/StringStream) from 0.0.5 to 0.0.6.
- [Release notes](https://github.com/mhart/StringStream/releases)
- [Commits](mhart/StringStream@v0.0.5...v0.0.6)

Signed-off-by: dependabot[bot] <support@github.com>

* upgrading redis

Co-authored-by: Jake <jakesyl@gmail.com>
Co-authored-by: Matthew Oaxaca <mattoaxaca@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump handlebars from 4.0.11 to 4.5.3 (#35)

* Add link to node redis library

* Bump stringstream from 0.0.5 to 0.0.6 (#27)

Bumps [stringstream](https://github.com/mhart/StringStream) from 0.0.5 to 0.0.6.
- [Release notes](https://github.com/mhart/StringStream/releases)
- [Commits](mhart/StringStream@v0.0.5...v0.0.6)

Signed-off-by: dependabot[bot] <support@github.com>

* Bump handlebars from 4.0.11 to 4.5.3

Bumps [handlebars](https://github.com/wycats/handlebars.js) from 4.0.11 to 4.5.3.
- [Release notes](https://github.com/wycats/handlebars.js/releases)
- [Changelog](https://github.com/wycats/handlebars.js/blob/master/release-notes.md)
- [Commits](handlebars-lang/handlebars.js@v4.0.11...v4.5.3)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: Jake <jakesyl@gmail.com>
Co-authored-by: Matthew Oaxaca <mattoaxaca@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump eslint from 4.17.0 to 4.18.2 (#31)

* Add link to node redis library

* Bump stringstream from 0.0.5 to 0.0.6 (#27)

Bumps [stringstream](https://github.com/mhart/StringStream) from 0.0.5 to 0.0.6.
- [Release notes](https://github.com/mhart/StringStream/releases)
- [Commits](mhart/StringStream@v0.0.5...v0.0.6)

Signed-off-by: dependabot[bot] <support@github.com>

* Bump sshpk from 1.13.1 to 1.16.1 (#33)

Bumps [sshpk](https://github.com/joyent/node-sshpk) from 1.13.1 to 1.16.1.
- [Release notes](https://github.com/joyent/node-sshpk/releases)
- [Commits](TritonDataCenter/node-sshpk@v1.13.1...v1.16.1)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump eslint from 4.17.0 to 4.18.2

Bumps [eslint](https://github.com/eslint/eslint) from 4.17.0 to 4.18.2.
- [Release notes](https://github.com/eslint/eslint/releases)
- [Changelog](https://github.com/eslint/eslint/blob/master/CHANGELOG.md)
- [Commits](eslint/eslint@v4.17.0...v4.18.2)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: Jake <jakesyl@gmail.com>
Co-authored-by: Matthew Oaxaca <mattoaxaca@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump js-yaml from 3.10.0 to 3.13.1 (#30)

* Add link to node redis library

* Bump stringstream from 0.0.5 to 0.0.6 (#27)

Bumps [stringstream](https://github.com/mhart/StringStream) from 0.0.5 to 0.0.6.
- [Release notes](https://github.com/mhart/StringStream/releases)
- [Commits](mhart/StringStream@v0.0.5...v0.0.6)

Signed-off-by: dependabot[bot] <support@github.com>

* Bump sshpk from 1.13.1 to 1.16.1 (#33)

Bumps [sshpk](https://github.com/joyent/node-sshpk) from 1.13.1 to 1.16.1.
- [Release notes](https://github.com/joyent/node-sshpk/releases)
- [Commits](TritonDataCenter/node-sshpk@v1.13.1...v1.16.1)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump js-yaml from 3.10.0 to 3.13.1

Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 3.10.0 to 3.13.1.
- [Release notes](https://github.com/nodeca/js-yaml/releases)
- [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md)
- [Commits](nodeca/js-yaml@3.10.0...3.13.1)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: Jake <jakesyl@gmail.com>
Co-authored-by: Matthew Oaxaca <mattoaxaca@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump lodash from 4.17.5 to 4.17.15 (#28)

* Add link to node redis library

* Bump stringstream from 0.0.5 to 0.0.6 (#27)

Bumps [stringstream](https://github.com/mhart/StringStream) from 0.0.5 to 0.0.6.
- [Release notes](https://github.com/mhart/StringStream/releases)
- [Commits](mhart/StringStream@v0.0.5...v0.0.6)

Signed-off-by: dependabot[bot] <support@github.com>

* Bump sshpk from 1.13.1 to 1.16.1 (#33)

Bumps [sshpk](https://github.com/joyent/node-sshpk) from 1.13.1 to 1.16.1.
- [Release notes](https://github.com/joyent/node-sshpk/releases)
- [Commits](TritonDataCenter/node-sshpk@v1.13.1...v1.16.1)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump lodash from 4.17.5 to 4.17.15

Bumps [lodash](https://github.com/lodash/lodash) from 4.17.5 to 4.17.15.
- [Release notes](https://github.com/lodash/lodash/releases)
- [Commits](lodash/lodash@4.17.5...4.17.15)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: Jake <jakesyl@gmail.com>
Co-authored-by: Matthew Oaxaca <mattoaxaca@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Dependency Update & Lint

* Feature/Proxy Implementation (#41)

* WIP Proxy Implementation

* WIP Test Coverage

* Fixing Tests

Co-authored-by: Jason <jason@step.com>
Co-authored-by: Sergey Kazantsev <35541456+ProtonGustave@users.noreply.github.com>
Co-authored-by: Jake <jakesyl@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Chris Peddecord <cpeddecord@gmail.com>
@moaxaca moaxaca mentioned this pull request May 25, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@ThisIsMissEm ThisIsMissEm ThisIsMissEm left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jd20 @coveralls @moaxaca @ThisIsMissEm