-
Notifications
You must be signed in to change notification settings - Fork 492
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
internal/cli: add db.engine flag #964
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #964 +/- ##
===========================================
+ Coverage 57.08% 57.12% +0.04%
===========================================
Files 633 633
Lines 77324 77332 +8
===========================================
+ Hits 44141 44179 +38
+ Misses 29417 29394 -23
+ Partials 3766 3759 -7
☔ View full report in Codecov by Sentry. |
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.
Few nit picks before merging.
- Run
make docs
to update the docs at the end. - Update the 9
config.toml
files with this value (commented) (inbuilder
andpackaging
modules). - Update
example_config.toml
indocs/cli
. - Test it locally to confirm few things like - if pebble db is enabled for fresh node or not, if you start node with leveldb and restart it with pebble are the errors handled correctly or not, etc.
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, let's wait for the CI to pass.
Description
With recent changes in geth a new flag was introduced to select upon the database. We already have the required changes after upstream merge, I've just added the flag to select the database.
Changes
Breaking changes
N.A
Nodes audience
N.A
Checklist
Cross repository changes
Testing
Manual tests
I've tested it locally and it works well.
Additional comments
POS-1763