-
Notifications
You must be signed in to change notification settings - Fork 685
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: Merge Testbed
and RuntimeTestbed
#8242
Conversation
As the separation was not bringing much value and made existing code more complex. See near#8201 for full motivation.
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! I left a minor comment on formatting style. Please address it and then afterwards you can either press the squash-and-merge button or add the label "S-auto-merge" so the bot will auto merge for you. (Useful when someone else merges while CI still runs on your PR)
use std::collections::HashMap; | ||
use std::sync::Arc; | ||
|
||
use near_primitives::transaction::SignedTransaction; | ||
use near_primitives::receipt::Receipt; | ||
use near_primitives::runtime::config_store::RuntimeConfigStore; | ||
use near_primitives::runtime::migration_data::{MigrationData, MigrationFlags}; | ||
use near_primitives::test_utils::MockEpochInfoProvider; | ||
use near_primitives::transaction::{ExecutionStatus, SignedTransaction}; | ||
use near_primitives::types::{Gas, MerkleHash}; | ||
use near_primitives::version::PROTOCOL_VERSION; | ||
|
||
use near_store::{ShardTries, ShardUId, Store, StoreCompiledContractCache}; | ||
use near_store::{TrieCache, TrieCachingStorage, TrieConfig}; | ||
use near_vm_logic::ExtCosts; | ||
|
||
use near_vm_logic::{ExtCosts, VMLimitConfig}; | ||
|
||
use node_runtime::{ApplyState, Runtime}; | ||
|
||
use crate::config::{Config, GasMetric}; | ||
use crate::gas_cost::GasCost; | ||
use crate::testbed::RuntimeTestbed; | ||
use genesis_populate::get_account_id; | ||
use genesis_populate::state_dump::StateDump; |
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.
Nitpick: Grouping of imports is supposed to be in one block (See internal style guide)
We are not super strict about it but I like to point it out at least once when people join.
On a side note, if you are using rust-analyzer, I recommend enabling "rust-analyzer.assist.importGranularity": "module"
which is also mentioned in the style guide. It will not help with empty lines between groups but it will help with the group nesting.
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.
Good point, looks like rust-lang/rustfmt#5083 is still unstable, sigh...
I'm indeed using rust-analyzer with this setting enabled already (that prompts the question which other settings I should set, will raise it on Zulip)
As the separation was not bringing much value and made existing code more complex.
See #8201 for full motivation.