search

basho-labs / riak-c-client

Riak C Client
Apache License 2.0
20 stars 8 forks source link

Add Missing get/set Accessors and CUnit tests #27

Closed hazen closed 10 years ago

hazen commented 10 years ago

Addresses #14 and #10

rzezeski commented 10 years ago

It's going to take me a while to review this, but good news is it builds on OSX and passes make check.

rzezeski commented 10 years ago

TL;DR: +1 to merge.

Okay, as 2.0 approaches it's getting obvious that I just wont have time to properly finish reviewing this PR. It's just too big and I'm too new to both C and this client to review this with any speed. In fact, I think I should play with the client a bit more before any future reviews so I can get a better feeling for it. It would also be nice if future patches aren't this large. Many of my comment above still stand but I'll leave it up to you if you want to act on them. Since we are just trying to get something out and this is blocking other PRs I'm fine with seeing it merged and implementation details can be hashed out later when there is more time to discuss.

hazen commented 10 years ago

Sounds good, @rzezeski. Thanks for all the effort you already put in. I'll address what I can, but a deeper dive sounds like a great next step once I have the initial version released.