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

Ready: Add isNotFound parameter to data type responses #654

Merged
merged 3 commits into from
Aug 9, 2016

Conversation

alexmoore
Copy link
Contributor

@alexmoore alexmoore commented Aug 5, 2016

Fixes #536 (CLIENTS-552).

@alexmoore alexmoore changed the title Not Ready: Add isNotFound parameter to data type responses Ready: Add isNotFound parameter to data type responses Aug 5, 2016
@alexmoore alexmoore added this to the riak-java-client-2.0.7 milestone Aug 5, 2016

private final Map<BinaryValue, List<RiakDatatype>> entries =
new HashMap<BinaryValue, List<RiakDatatype>>();
private final Map<BinaryValue, List<RiakDatatype>> entries = new HashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this change, why is it in with a PR that adds a NotFound field to the fetch response?

Copy link
Contributor

Choose a reason for hiding this comment

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

Pretty sure this is a resharper code standards format change. He is updating files as he touches them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, just a general safe refactoring to standardize spacing, formatting etc as I touch files. I should have moved that specific one into a separate commit but probably saw red at the mixing of tabs and spaces.

@russelldb
Copy link
Member

russelldb commented Aug 9, 2016

The code looks good.

One question: there seems to be at least 2 things in this PR 1. the change as described 2. a general refactor.

I think 2 PRs would have been better.

@russelldb
Copy link
Member

I can't get the iTests to run, I just get java.lang.AssertionError: Update operation failed: com.basho.riak.client.core.NoNodesAvailableException even after using java.lang.AssertionError: Update operation failed: com.basho.riak.client.core.NoNodesAvailableException. I can ping the nodes, and member-status says they're a cluster and happy.

Will wait for @alexmoore to get in rather than persist at this point

@christophermancini
Copy link
Contributor

👍 Tests pass, code looks good.

@alexmoore
Copy link
Contributor Author

@russelldb I'll try to at least split them into different commits in the future. I've been avoiding a "mega-refactor" PR, just because it would be terrible to review everything all at once. reformatting as I go seems to work though.

* @return a Long value for this RiakCounter.
*/
@Override
@Override
public Long view()
Copy link
Contributor

@srgg srgg Aug 9, 2016

Choose a reason for hiding this comment

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

Why don't us change Long to long to avoid Boxing..

UPD: it turned out that this Long is dropping from RiakDatatype, therefore IMO better to change:

 private long value = 0;

to use Long instead of long, and maybe even to make RiakDatatype be a Generic class

@russelldb
Copy link
Member

+1

Tests pass. Looks good. WFM

@alexmoore alexmoore merged commit bb185ad into develop Aug 9, 2016
@alexmoore alexmoore deleted the 536-detect-non-existing-crdts branch August 9, 2016 18:49
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.

4 participants