-
Notifications
You must be signed in to change notification settings - Fork 39
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
refactor: the all-in-one file of core-run
#1349
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cc @Simon-Tl The prometheus service already applied the same change in #1348. -- update by @yangby-cryptape |
yangby-cryptape
force-pushed
the
yangby/refactor-core-run
branch
from
August 29, 2023 15:46
d1154c5
to
7aad3a2
Compare
yangby-cryptape
changed the title
WIP refactor: the all-in-one file of
refactor: the all-in-one file of Aug 29, 2023
core-run
core-run
yangby-cryptape
force-pushed
the
yangby/refactor-core-run
branch
from
August 29, 2023 16:01
7aad3a2
to
62d01e3
Compare
yangby-cryptape
requested review from
Flouse,
driftluo and
KaoImin
and removed request for
felicityin
August 29, 2023 16:05
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as outdated.
This comment was marked as outdated.
yangby-cryptape
force-pushed
the
yangby/refactor-core-run
branch
from
August 31, 2023 08:30
62d01e3
to
68e2b0e
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
CI tests run on commit:
CI test list:
Please check ci test results later. |
KaoImin
approved these changes
Aug 31, 2023
driftluo
approved these changes
Sep 1, 2023
Flouse
approved these changes
Sep 1, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it?
This PR refactors the all-in-one file of
core-run
:Split the all-in-one file of
core-run
into several files.The original file is the 2nd largest file in the whole repository.
It has 1000+ lines and 70+ lines for importing items.
Split the "huge" struct
Axon
.Examples:
👉 Disallow "one-time" field, i.e., don't save intermediate data in global.
Should we have to refresh the fields that only used once?
We don't have to concern that if there is no "one-time" fields.
In
Axon
, thespec
(only has theaccounts
field),genesis
andstate_root
are "one-time" fields.They are only used when boot the services.
axon/core/run/src/lib.rs
Lines 84 to 90 in 0ed2ce6
Especially
state_root
, it's just an intermediate data which only used once.And, it can be replaced with
self.genesis.block.header.state_root
.axon/core/run/src/lib.rs
Line 175 in 0ed2ce6
axon/core/run/src/lib.rs
Lines 575 to 579 in 0ed2ce6
👉 If a method only use
self.aaa.bbb.ccc
, then should not useself
as the parameter.This will result in that's hard to detect minimal dependencies for class methods, so that:
Self
with many dummy fields.Remove all default unspecified addresses.
Examples:
👉 The
jaeger
service listens0.0.0.0:6831
as default.axon/core/run/src/lib.rs
Lines 764 to 767 in 0ed2ce6
Remove lots of unnecessary
unrwap()
.Examples:
👉 The caller function already handle the errors.
axon/core/run/src/lib.rs
Lines 739 to 755 in 0ed2ce6
axon/core/run/src/lib.rs
Lines 254 to 259 in 0ed2ce6
Use latest block if possible.
👉 When initialize
MetaData
.The following code is executed when axon starts:
axon/core/run/src/lib.rs
Lines 316 to 323 in 2877191
Modified as the following code:
👉 When initialize
MemPool
.The following code is executed when axon starts:
axon/core/run/src/lib.rs
Lines 436 to 445 in 2877191
Modified as the following code:
Major Changes
The jaeger service won't be started if no
tracing_address
is set in the configuration file.No default address anymore.
What is the impact of this PR?
✅ No Breaking Change
PR relation:
Ref Split sub-command
run
toinit
andrun
#1345Prepare for the feature in that issue, so that refactor and features are not mixed in one commit.
CI Settings
CI Usage
Tip: Check the CI you want to run below, and then comment
/run-ci
.CI Switch
CI Description
cargo clippy --all --all-targets --all-features
cargo +nightly fmt --all -- --check
andcargo sort -gwc