Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Move config path generation into the service config for reusability #3978

Merged
gavofyork merged 8 commits into from
Nov 1, 2019
Merged

Move config path generation into the service config for reusability #3978

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4ecbc2b
Move config path generation into the service config for reusability
gnunicorn Oct 23, 2019
fc9f1b0
Make NoCostum Default and fix tests
gnunicorn Oct 31, 2019
8a1a25d
Apply suggestions from code review
gnunicorn Nov 1, 2019
3f12f75
remove function not used anymore
gnunicorn Nov 1, 2019
7be8cbf
Merge remote-tracking branch 'origin/master' into ben-make-config-pat…
gnunicorn Nov 1, 2019
0113741
Make path into an option
gnunicorn Nov 1, 2019
e31e204
remove database_path function and call it directly
gnunicorn Nov 1, 2019
bd0f5de
remove helper functions, use consts
gnunicorn Nov 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Apply suggestions from code review 
Co-Authored-By: Bastian Köcher <[email protected]>
  • Loading branch information
@gnunicorn @bkchr
gnunicorn and bkchr authored Nov 1, 2019
commit 8a1a25d9741616fd06c3ca645dae7d450a8b2e8c
9 changes: 4 additions & 5 deletions core/cli/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ pub struct ParseAndPrepareBuildSpec<'a> {

impl<'a> ParseAndPrepareBuildSpec<'a> {
/// Runs the command and build the chain specs.
pub fn run<C,G, S, E>(
pub fn run<C, G, S, E>(
self,
spec_factory: S
) -> error::Result<()> where
Expand All @@ -320,7 +320,7 @@ impl<'a> ParseAndPrepareBuildSpec<'a> {
let raw_output = self.params.raw;
let mut spec = load_spec(&self.params.shared_params, spec_factory)?;

if spec.boot_nodes().is_empty() {
if spec.boot_nodes().is_empty() && !self.params.disable_default_bootnode {
let base_path = base_path(&self.params.shared_params, self.version);
let config = service::Configuration::<C,_,_>::default_with_spec_and_base_path(spec.clone(), base_path);
let node_key = node_key_config(self.params.node_key_params, &Some(config.network_path()))?;
Expand Down Expand Up @@ -571,8 +571,7 @@ fn fill_transaction_pool_configuration<C, G, E>(
}

/// Fill the given `NetworkConfiguration` by looking at the cli parameters.
fn fill_network_configuration
(
fn fill_network_configuration(
cli: NetworkConfigurationParams,
config_path: PathBuf,
config: &mut NetworkConfiguration,
Expand Down Expand Up @@ -680,7 +679,7 @@ where
)?
}

config.keystore_path = cli.keystore_path.unwrap_or_else( || config.in_chain_config_dir("keystore"));
config.keystore_path = cli.keystore_path.unwrap_or_else(|| config.in_chain_config_dir("keystore"));

config.database = DatabaseConfig::Path {
path: config.database_path(),
Copy link
Contributor

@tomaka tomaka Nov 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the change yet. I'm commenting in a hurry because you're about to merge.

This line is kind of a red flag. We store config.database_path() in config.database.path.

Copy link
Contributor Author

@gnunicorn gnunicorn Nov 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the general change of this PR is to move the generation of relative paths from the internal *_path(&base_path-style onto the configuration object. database_path() there is only a handy alias for self.base_path.clone().join("database") on config.

What exactly do you consider the red flag?

Expand Down
1 change: 0 additions & 1 deletion core/service/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ impl<C, G, E> Configuration<C, G, E> where
}

impl<C, G, E> Configuration<C, G, E> {

/// Returns full version string of this configuration.
pub fn full_version(&self) -> String {
full_version_from_strs(self.impl_version, self.impl_commit)
Expand Down