Skip to content

Implement bitpos command. #412

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

Merged
merged 2 commits into from
May 7, 2014
Merged

Implement bitpos command. #412

merged 2 commits into from
May 7, 2014

Conversation

badboy
Copy link
Contributor

@badboy badboy commented Feb 27, 2014

This command landed in unstable today.
Tests are taken and translated from the original Tcl test suite.

This command landed in unstable today.
Tests are taken and translated from the original Tcl test suite.
synchronize do |client|
command = [:bitpos, key, bit]
command << start if start
command << stop if stop
Copy link
Contributor

Choose a reason for hiding this comment

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

If start is not specified, the value given for stop will be injected into start's position; it would be wise to safeguard against this:

Consider:

# attempt to get first 3 bytes
redis.bitpos('foo',0,nil,3)
# intent: BITPOS foo 0 0 3
# actual: BITPOS foo 0 3
# actually get bytes after 3rd byte

Maybe something like this:

def bitpos(key, bit, start=nil, stop=nil)
  if stop and not start
    raise(ArgumentError, 'stop parameter specified without start parameter')
  end
  # ...
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, will fix it that way.

If start is not specified, the value given for stop will be injected
into start's position; it is wise to safeguard against this.
(via @yaauie)
@badboy
Copy link
Contributor Author

badboy commented May 6, 2014

Somehow the redis instance for testing died in between, that's why the test is failing. Could someone with access rights re-trigger that build? (or give me access rights, cough cough)

@yaauie
Copy link
Contributor

yaauie commented May 7, 2014

I restarted the offending job & will check back on it later.

yaauie added a commit that referenced this pull request May 7, 2014
Implement bitpos command.
@yaauie yaauie merged commit 9c45c6c into redis:master May 7, 2014
@badboy
Copy link
Contributor Author

badboy commented May 7, 2014

Great! :)

@yaauie yaauie added this to the v3.1 milestone Jun 4, 2014
@yaauie
Copy link
Contributor

yaauie commented Jun 6, 2014

This feature was released in 3.1.0; thanks for your contribution 👍

@badboy
Copy link
Contributor Author

badboy commented Jun 6, 2014

Yippieh!

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