Firstly, but I think that the way modules are structured is rather unidiomatic and makes the code hard to read. For example:

use juliaup::{
    command_selfchannel::run_command_selfchannel,
    command_selfuninstall::run_command_selfuninstall,
    command_config_backgroundselfupdate::run_command_config_backgroundselfupdate,
    command_config_startupselfupdate::run_command_config_startupselfupdate,
    command_config_modifypath::run_command_config_modifypath,
};

The code is not fully utilising the module system, and this looks like the naming conventions used in C, which has header hell and no module system, so you have to prefix everything. That's not needed in Rust.

So, this import could be simplified to:

use juliaup::command::{
    self_channel,
    self_uninstall,
    config::{
        background_self_update,
        startup_self_update,
        modify_path,
    },
};

You can immediately perceive the difference in readability.

And then, while using it:

// example
self_channel::run();

Second, the code is not formatted properly, with snippets like this:

fn get_juliaup_home_path() -> Result<PathBuf> {
    let entry_sep = if std::env::consts::OS == "windows" {';'} else {':'};

    let path = match std::env::var("JULIA_DEPOT_PATH") {
        Ok(val) => {
            let path = PathBuf::from(val.to_string().split(entry_sep).next().unwrap()); // We can unwrap here because even when we split an empty string we should get a first element.

            if !path.is_absolute() {
                bail!("The `JULIA_DEPOT_PATH` environment variable contains a value that resolves to an an invalid path `{}`.", path.display());
            };

            path.join("juliaup")
        }
        Err(_) => {
    let path = dirs::home_dir()
        .ok_or(anyhow!(
            "Could not determine the path of the user home directory."
        ))?
        .join(".julia")
        .join("juliaup");

            if !path.is_absolute() {
                bail!(
                    "The system returned an invalid home directory path `{}`.",
                    path.display()
                );
            };

            path
        }
    };

    Ok(path)
}

This is the simplest to fix: a simple cargo fmt would do the trick. Plus, formatting checks should probably be added to CI.

Next, and this is rather subjective, but commands should probably be structs containing all that a command needs, and then a simple trait to run it:

pub trait Command {
    type Error;

    fn run(self) -> anyhow::Result<Self::Error>;
}

Which would be used like so:

use juliaup::command::{self, Command};

fn any() {
    command::Add::new(/* params */).run();
}

Lastly, linting with cargo clippy would also be rather helpful in catching unidiomatic code (use it in CI too).

I understand that these are massive changes, so take your time to debate them. I believe that these changes will make the codebase far more approachable.

Clean up codebase #313

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions