search

curiosity-ai / rocksdb-sharp

.net bindings for the rocksdb by facebook
BSD 2-Clause "Simplified" License
155 stars 38 forks source link

Support for Secondary RocksDB Instances #16

Closed roeeiit1 closed 2 years ago

roeeiit1 commented 2 years ago

Hi @theolivenbaum
Wanted to know if there is anything else you need me to add in order to complete this pull request 🙂 Thanks in advance

almogtavor commented 2 years ago

A very anticipated feature

theolivenbaum commented 2 years ago

Hi everyone! PR looks good, didn't know about this feature before. Think only thing to fix is to move the dependencies for testing to the test project and remove it from the library project

roeeiit1 commented 2 years ago

Hey, sure! Is there an existing test project or should I create one? Thank you for the quick response! :D

theolivenbaum commented 2 years ago

Apologies! I thought there was one - but was confusing with another one of our repos. If you could move it then to a separate project, that would be better, so we don't introduce the test SDKs as dependencies of the library! Thanks!

almogtavor commented 2 years ago

Why would let the tests be in the same repo? This doesn't mean they have to be included in the SDK

roeeiit1 commented 2 years ago

No no, I think he means to leave them in the repo just put them in another project as the project is closed as the package in the end :)

roeeiit1 commented 2 years ago

@theolivenbaum Done! :) btw i also added support for compiling the dll for dotnet core 3.1 if thats ok with you as i dont see anything that makes it conflict there and i am still using dotnet core 3.1 in my app 😅 can remove that if needed or open a sperate pr for it

almogtavor commented 2 years ago

Looks great to me

theolivenbaum commented 2 years ago

Merged!