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

Improve RocksDB interface #23

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

jurmous
Copy link

@jurmous jurmous commented Oct 22, 2019

  • Add keyMayExist methods
  • Add file deletion methods
  • Add background work pause and continue methods
  • Fix spelling mistakes

@jurmous jurmous force-pushed the Improve-RocksDB-interface branch from f1dd03e to 3889886 Compare October 22, 2019 18:10
- Add keyMayExist methods
- Add file deletion methods
- Add background work pause and continue methods
- Fix spelling mistakes
@jurmous jurmous force-pushed the Improve-RocksDB-interface branch from 3889886 to 7a74815 Compare October 22, 2019 19:59
@iabudiab
Copy link
Owner

Hey @jurmous, I'll take a look at this tomorrow evening and will release a new version after the pull. I left a small feedback there in regards to the API, let's discuss it tomorrow in more detail.
Thanks again for all the PRs!

@jurmous
Copy link
Author

jurmous commented Oct 23, 2019

I don't see the feedback? Where can it be found?

Probably I would like to expand this PR to add more methods to the RocksDB interface. Could also be a separate PR. Maybe it is good to wait a bit for a release so more methods can be added. I am still playing around to use this library as the iOS/Mac interface into a RocksDB Kotlin multiplatform module.

Thanks for merging the others! And all the work in creating this library! 😄

@iabudiab
Copy link
Owner

I don't see the feedback? Where can it be found?

Sorry my bad, I totally forgot about the new way GitHub starts reviews. I forgot to submit it.

*/
- (BOOL)keyMayExist:(NSData *)aKey
readOptions:(nullable void (^)(RocksDBReadOptions *readOptions))readOptions
value:(NSString * _Nullable *_Nullable)value;
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not really sure about the pass-by-ref parameter here. This breaks the trailing closure syntax and wouldn't translate nicely when used in Swift. Instead of returning BOOL how about returning a nullable NSString *.

@iabudiab
Copy link
Owner

If you want to expand upon this PR or open a new one, it would be much appreciated 😉

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