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

Expose all remaining read and write options via the C API #3185

Conversation

glittershark
Copy link
Contributor

@glittershark glittershark commented Nov 19, 2017

Expose read and write options via the C API

@maysamyabandeh
Copy link
Contributor

Thanks @glittershark. Can you expose all the options that are missing from the C API? It is better to do them all in one shot rather than having a separate PR for each.

@glittershark
Copy link
Contributor Author

Happy to!

Expose almost all remaining read and write options via the C API.

The only thing that's not here is table_filter, as it's not clear how to
allow C callers to pass `std::function`s.
@facebook-github-bot
Copy link
Contributor

@glittershark has updated the pull request.

@glittershark glittershark changed the title Expose prefix_same_as_start via the C API Expose all remaining read and write options via the C API Nov 21, 2017
@glittershark
Copy link
Contributor Author

@maysamyabandeh Updated to expose all options except ReadOptions.table_filter, as I didn't know how you wanted to allow C callers to pass std::functions - there's a little prior art here with merge operators, but as that's an implemented interface rather than a std::function I didn't want to move forward with it unilaterally.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@maysamyabandeh has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@sagar0 is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@sagar0 is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@sagar0 is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@glittershark glittershark deleted the c-expose-prefix-same-as-start branch November 28, 2017 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants