-
Notifications
You must be signed in to change notification settings - Fork 224
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
Serialization refactor step 1 #255
Conversation
I haven't reviewed yet, so if you like, you can rebase this on master (and force push) or merge in master. |
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.
Note that currently all modules under tendermint/src which contain submodules follow the current pattern:
module::submodule
in
tendermint/src/module/submodule.rs
submodule
is then included in module tree via:
tendermint/src/module.rs
which is then included in module tree via:
tendermint/src/lib.rs
Should we use this pattern here too? Currently all submodules are defined in
tendermint/src/serializers.rs
Step 3 does that but unfortunately the original work had dependencies which would've made this into the "big PR" again, if I implemented it 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 for splitting this up @greg-szabo 👍
I left a few minor comments. The PR looks good so far.
* rename/flatten serializer methods (primitives and time_duration) * Fix all clippy warnings: - `cargo clippy --all --all-targets -- -Dwarnings -Drust-2018-idioms`
Codecov Report
@@ Coverage Diff @@
## master #255 +/- ##
========================================
- Coverage 27.6% 27.6% -0.1%
========================================
Files 99 99
Lines 3651 3650 -1
Branches 1169 1168 -1
========================================
- Hits 1010 1009 -1
Misses 1831 1831
Partials 810 810
Continue to review full report at Codecov.
|
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 a lot @greg-szabo 👍
with
. (Looks nicer.)This is a first step in refactoring serialization outloined in issue #247 .
Soft-depends on Issue #249 / PR #256 for integration test success.