Skip to content
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

Consitent Read Included in find #47

Closed
wants to merge 1 commit into from
Closed

Consitent Read Included in find #47

wants to merge 1 commit into from

Conversation

prasathsarath
Copy link

This is for the specific issue #38
Also I couldn't find any doc/page related to contribution to repo, would be great if someone share them
Signed-off-by: sarathprasath prasath5s@live.com

Signed-off-by: sarathprasath <prasath5s@live.com>
@awood45
Copy link
Contributor

awood45 commented Dec 7, 2016

Fair point on the contributing guide, I'll look at writing one up.

As for this change, there are a few problems that prevent me from being able to take it:

  1. It's a breaking change - as mentioned in the issue, attributes called :consistent_read (which is pretty plausible) would break. This can be worked around, but users would see non-intuitive errors.
  2. This particular diff is broken, and unit tests would show this. :consistent_read would appear in both the key (which would be an error), and in the pass-through attribute you provide. This would likely lead to breakage as the code above isn't hardened against this.
  3. Any solution needs to account for the other #get_item parameters that users may want to pass through to the call. The solution would probably be a method called #find_with_options or similar that has a different signature.

I appreciate the PR and will take this as a big +1 on prioritizing this feature. Thanks!

@awood45 awood45 closed this Dec 7, 2016
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.

2 participants