-
Notifications
You must be signed in to change notification settings - Fork 55
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
remove dbs no one uses #116
Conversation
faddat
commented
Jan 7, 2024
- add pebbledb
- use latest pebble
- added a ton of test rigor and fixed linting
- if always false we don't need the param
- really we don't need that
- remove databases no one uses
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #116 +/- ##
==========================================
- Coverage 68.59% 60.70% -7.89%
==========================================
Files 27 18 -9
Lines 2124 1438 -686
==========================================
- Hits 1457 873 -584
+ Misses 592 506 -86
+ Partials 75 59 -16
|
I am okay with codecov propblems. We should rip it out anyhow. |
Hi @faddat , thank you for the benchmarks and insights. As @adizere stated, we don't want to abruptly remove support for all other databases as we have a very large userbase still using them. We have plans to converge to one DB backend in the future but this requires an extensive study on what users are using. I would therefore separate the concerns of adding Pebble and removing other backends and keep in the PR only the addition of Pebble. |
ok, I will change this to draft in that case. |
Just so you know, somebody has written a universal converter. I just don't know who. It may have been The team that starts on a p but isn't persistence but I always forget their name |
personally I dont care about tenths of a percent of on codecov. Please let me know if it matters. |
I think that this PR remains a good idea. |
The PR would be a lot better without this commit a574117. It's difficult to review right now. Can you revert that commit? |
100% |
closing in favor of #115 |