-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
use DBBackend set at compile time #3792
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #3792 +/- ##
===========================================
- Coverage 61% 60.97% -0.04%
===========================================
Files 191 191
Lines 14174 14182 +8
===========================================
Hits 8647 8647
- Misses 4973 4981 +8
Partials 554 554 |
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.
Needs PENDING.md
, a question about error handling.
Also, benchmarks would be interesting.
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.
TestedACK. Needs a pending log entry. Also, I don't see why we should be supporting user defined ldflags
? This should be only used internally by the makefile as I cannot see a circumstance where these would need to be overwritten? Do you have an example?
EDIT: Also, CI needs a kick.
@@ -40,6 +40,9 @@ tags. | |||
|
|||
### SDK | |||
|
|||
* [\#3719](https://github.com/cosmos/cosmos-sdk/issues/3719) DBBackend can now be set at compile time. |
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.
@cwgoes @alexanderbez pending line is here
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.
Thanks!
@alexanderbez dixit:
They wouldn't be overwritten, but appended. Same goes for build tags. |
Users might want custom build options - it seems unlikely that they would define custom build flags by accident - is there a reason not to give them the option? |
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.
ACK
@alexanderbez is it a blocker then? Traditionally, open source programs' build systems allow user to override {C,{,XX,PP}LD}FLAGS. I think that allowing users to customise their build does not do any harm |
without this change does previous level db performance test result still hold...? Would it possible your previous performance test run against goleveldb... |
Replace NewGoLevelDB() with generic NewLevelDB() calls.
DB backend is picked according to build time configuration.
Makefile:
adding WITH_CLEVELDB=yes to make install.
Closes: #3719
docs/
)PENDING.md
with issue #Files changed
in the github PR explorerFor Admin Use: