-
Notifications
You must be signed in to change notification settings - Fork 27
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
chore(sequencer_node): remove chain_id from config #1710
Conversation
ec07bc8
to
ebe5117
Compare
1424f3d
to
a075274
Compare
ebe5117
to
e9091ce
Compare
e9091ce
to
0213453
Compare
a075274
to
47f1e1a
Compare
0213453
to
d64c8e5
Compare
47f1e1a
to
cfa79b7
Compare
d64c8e5
to
c46001b
Compare
cfa79b7
to
96126cb
Compare
384612a
to
36cf61d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1710 +/- ##
===========================================
+ Coverage 40.10% 50.65% +10.54%
===========================================
Files 26 286 +260
Lines 1895 31912 +30017
Branches 1895 31912 +30017
===========================================
+ Hits 760 16164 +15404
- Misses 1100 14619 +13519
- Partials 35 1129 +1094 ☔ 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.
Reviewable status: 0 of 6 files reviewed, 1 unresolved discussion (waiting on @Itay-Tsabary-Starkware)
crates/mempool_node/src/config/node_config.rs
line 64 at r2 (raw file):
/// The [chain id](https://docs.rs/starknet_api/latest/starknet_api/core/struct.ChainId.html) of the Starknet network. #[validate(custom = "validate_ascii")] pub chain_id: ChainId,
Why not adding the required params as a field 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.
Reviewable status: 0 of 6 files reviewed, 1 unresolved discussion (waiting on @nadin-Starkware)
crates/mempool_node/src/config/node_config.rs
line 64 at r2 (raw file):
Previously, nadin-Starkware (Nadin Jbara) wrote…
Why not adding the required params as a field here?
They are implicitly added as pointer params. Let's further discuss f2f.
commit-id:5d71d99c
36cf61d
to
6722252
Compare
Artifacts upload triggered. View details 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.
Reviewed 3 of 6 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware)
✓ Commit merged in pull request #1725 |
Stack: