Skip to content

Updated commands.json to the version of 200160226 #52

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

Closed
wants to merge 1 commit into from

Conversation

qrilka
Copy link
Contributor

@qrilka qrilka commented Feb 27, 2016

As it's a quite a heavy patch I think we'll need to review it thoroughly
Also I see what could be improved (some commands could easily be fully supported e.g.)
And for sure we'll need a major version bump for this.

@qrilka qrilka force-pushed the commands-20160226 branch 3 times, most recently from 85f9feb to e6a9fdb Compare February 28, 2016 18:48
@qrilka qrilka mentioned this pull request Mar 1, 2016
@Thooms
Copy link
Contributor

Thooms commented Mar 10, 2016

Perhaps it could be wise to write in the same time the unit tests for everything? That way when the big change comes (I understand @informatikr wanted to go for a full handwritten API instead of generated code), we can go more smoothly?
I can review the patch this week and write the unittests if @informatikr or @k-bx are willing to merge afterwards :)

@k-bx
Copy link
Collaborator

k-bx commented Mar 10, 2016

@Thooms this would be highly appreciated. On my side, I started checking this PR against my work-project to make sure at least basic stuff works on tests/staging (maybe even prod for load-testing, but we'll see).

@Thooms
Copy link
Contributor

Thooms commented Mar 10, 2016

OK, I'm on it then. I'll try to write as much unittests as I can, let's see afterwards for the rest!

@qrilka
Copy link
Contributor Author

qrilka commented Mar 10, 2016

that sounds good @Thooms - do you want to put those unittests into this PR?

@Thooms
Copy link
Contributor

Thooms commented Mar 10, 2016

@qrilka I would prefer starting a new branch (since we're not only talking about updating the API anymore), and cherry-picking your commit into it, if you're okay with that.

@qrilka
Copy link
Contributor Author

qrilka commented Mar 10, 2016

Sure, other option is to merge this one and unittests as another PR and not do any releases until we have good coverage.
But @k-bx is the maintainer of this package

@k-bx
Copy link
Collaborator

k-bx commented Mar 10, 2016

I think cherry-picking should provide a good way to have these PRs separately implemented, but once Thooms's is ready, I can merge this one and Thooms's then, should work well.

@Thooms
Copy link
Contributor

Thooms commented Mar 23, 2016

Hello there, just a small message to signal I'm still working on the testsuite. There a lot of code to write, and I can't commit for now, but I'll make a PR as soon as it's showable :)

@Thooms
Copy link
Contributor

Thooms commented Mar 23, 2016

-> #61

@qrilka qrilka force-pushed the commands-20160226 branch from e6a9fdb to d0dd9f2 Compare March 24, 2016 20:12
@qrilka
Copy link
Contributor Author

qrilka commented Mar 24, 2016

Updated (actually rebased) according to #62 - it appears that Redis lists even commands from unstable version and they don't have "since" set.
@Thooms I suppose you'll need to rebase you work unfortunately

@Thooms
Copy link
Contributor

Thooms commented Mar 24, 2016

Will do, no problem!

@qrilka qrilka mentioned this pull request Mar 27, 2016
@k-bx k-bx closed this Mar 28, 2016
@k-bx
Copy link
Collaborator

k-bx commented Mar 28, 2016

I've merged this branch into commands-20160226 branch in the main hedis repo, thank you! Let's continue working on top of that one everyone.

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