Skip to content
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

improve: write modified config values back to the maker config.toml file #246

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

wthrajat
Copy link
Collaborator

        let mut config = MakerConfig::new(Some(&data_dir.join("config.toml")))?;

        if let Some(port) = port {
            config.port = port;
        }

        if let Some(rpc_port) = rpc_port {
            config.rpc_port = rpc_port;
        }

        if let Some(socks_port) = socks_port {
            config.socks_port = socks_port;
        }

        if let Some(connection_type) = connection_type {
            config.connection_type = connection_type;
        }

Maker config parameters like port, rpc_port, socks_port, connection_type when updated with new values, does NOT reflect in the config.toml file.
This PR aims to fix that.

Signed-off-by: wthrajat <rajatkhanduri290102@gmail.com>
Signed-off-by: wthrajat <rajatkhanduri290102@gmail.com>
Signed-off-by: wthrajat <rajatkhanduri290102@gmail.com>
@wthrajat wthrajat self-assigned this Aug 26, 2024
Signed-off-by: wthrajat <rajatkhanduri290102@gmail.com>
@wthrajat
Copy link
Collaborator Author

Clippy has been recently updated with a new lint: Too long first doc paragraph as described in doc_mod.rs.
Pushed the fix in 7d3ff35

@mojoX911
Copy link

@wthrajat can you make the long doc fix in a separate PR and I will merge it quickly. This error will trickle into other PRs too.

src/maker/api.rs Outdated Show resolved Hide resolved
Signed-off-by: wthrajat <rajatkhanduri290102@gmail.com>
Signed-off-by: wthrajat <rajatkhanduri290102@gmail.com>
@Shourya742
Copy link

@wthrajat Can you squeeze the commits.

src/taker/api.rs Outdated Show resolved Hide resolved
Signed-off-by: wthrajat <rajatkhanduri290102@gmail.com>
src/maker/config.rs Outdated Show resolved Hide resolved
Signed-off-by: wthrajat <rajatkhanduri290102@gmail.com>
Copy link

@mojoX911 mojoX911 left a comment

Choose a reason for hiding this comment

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

Ack.

Now that we have a write to file API, we don't need the separate write_default_config function. We can just create the default config and call write_to_file.

@@ -195,8 +195,8 @@ impl MakerConfig {
}

// Method to manually serialize the Maker Config into a TOML string
pub fn to_toml_string(&self) -> String {
format!(
pub fn update_maker_config(&self, file_path: &std::path::Path) {

Choose a reason for hiding this comment

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

Call this write_to_file. Because we can use this even when writing fresh config to a file. Not always update.

@mojoX911
Copy link

@wthrajat any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants