-
Notifications
You must be signed in to change notification settings - Fork 67
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
Change NewNative funcs to accept unsafe.Pointer #87
Conversation
Hello @muthukrishnan24 |
I agree not all structs need NewNative* exported. But some can be useful like SetNativeComparator |
@muthukrishnan24 yeah I agree with you. So lets update the PR: only expose NewNative for needed struct, hide others. |
Hi @linxGnu, updated the PR, exported NewNative for only needed structs |
8ae2770
to
83e2e7d
Compare
83e2e7d
to
4b89983
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for your great effort. Almost LGTM. Could you please address below comments
@@ -78,8 +79,8 @@ type MultiMerger interface { | |||
} | |||
|
|||
// NewNativeMergeOperator creates a MergeOperator object. | |||
func NewNativeMergeOperator(c *C.rocksdb_mergeoperator_t) MergeOperator { | |||
return &nativeMergeOperator{c} | |||
func NewNativeMergeOperator(c unsafe.Pointer) MergeOperator { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are exporting NewNative funcs for SliceTransform, CompactionFilter, MergeOperator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Great job. Thank you so much for this contribution
This PR changes NewNative functions to accept unsafe.Pointer instead of C pointers