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 naming of some of the new Options methods #82

Merged
merged 2 commits into from Oct 29, 2016
Merged

Improve naming of some of the new Options methods #82

merged 2 commits into from Oct 29, 2016

Conversation

kaedroho
Copy link
Contributor

@kaedroho kaedroho commented Oct 28, 2016

Part of #69

None of these methods have been in a past released version so I don't think we need to worry about backwards compatibility.

Changes as follows:

  • Renamed compression to set_compression_type. I think this is a much clearer name
  • Renamed add_merge_operator to set_merge_operator and add_comparator to set_comparator. Both of these methods replace the entire value of their respective option so "add" is the wrong term to use here.
  • Renamed set_block_cache_size_mb to rocksdb_options_optimize_for_point_lookup. To match RocksDB's API

@kaedroho
Copy link
Contributor Author

Ahh, totally forgot that add_merge_operator and add_comparator have been around for a while.

Should we provide a deprecation path? or scrap the renaming?

None of these methods have been in a past released version so I don't think we should worry about backwards compatibility.

Changes as follows:
 - Renamed ``compression`` to ``set_compression_type``. I think this is a much clearer name
 - Renamed ``add_merge_operator`` to ``set_merge_operator`` and ``add_comparator`` to ``set_comparator``. Both of these methods replace the entire value of their respective option so "add" is the wrong term to use here.
 - Renamed ``set_block_cache_size_mb`` to ``rocksdb_options_optimize_for_point_lookup``. To match RocksDB's API
@kaedroho
Copy link
Contributor Author

@space If you're happy with the renaming, we could keep the old versions around and decorate them with this: https://github.com/rust-lang/rfcs/blob/master/text/1270-deprecation.md#intended-use

@spacejam
Copy link
Member

Yeah! I'm in favor of having the old ones pass-through to the new ones, and have deprecation markings on them!

@kaedroho
Copy link
Contributor Author

Cheers @spacejam! Changes made in last commit

@spacejam
Copy link
Member

Thanks for this!

@spacejam spacejam merged commit 9b251d8 into rust-rocksdb:master Oct 29, 2016
@kaedroho kaedroho deleted the rename-options-methods branch October 29, 2016 14:32
BusyJay pushed a commit to BusyJay/rust-rocksdb that referenced this pull request Jul 7, 2017
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.

None yet

2 participants