derekdreery / depgraph

makefile-style dependency building in rust
5 stars 1 forks source link

API inconvenience #1

Closed Dushistov closed 7 years ago

Dushistov commented 7 years ago

I thought about creation of similar crate, but because of your already exists, I think I can use it, but there are several API inconvenience for me. Can you consider to improve them?

Context:

    let mut graph = depgraph::DepGraphBuilder::new();
    graph.add_rule(out_dir.join("out/path/file.o").to_str().unwrap(),
        &["src/input_file.asm"],
        build_assembly);
    let graph = graph.build().unwrap();
    graph.make(false).unwrap();
  1. Builder API. From my point of view, it should me more declarative (to increase readbility), like https://github.com/colin-kiegel/rust-derive-builder generate:
    depgraph::DepGraphBuilder::new()
        .add_rule("out/path/file.o", &["src/input_file.asm"], |x, y| Ok(()))
        .build()
        .expect("Can not crate dependcy graph")
        .make(false)
        .expect("Build failed");
  2. Rust has strong type checking, why not use it? I mean add_rule should show in clear way that it deals with file paths, so Path is clear way to do it, like this: add_rule(input: P, output: &[P]) where P: AsRef<Path>. This make code also self explainable.
  3. Make make parameter more clear with enum, like:
    enum MakeParams {
    None,
    ForceBuild
    }

    make(MakeParams::None) for me is much more clear, then make(false)

derekdreery commented 7 years ago

I think for (3) we should have method make that doesn't force (takes no params), and new method make_force that forces build. Otherwise looks like something that should be done!

Dushistov commented 7 years ago

I think for (3) we should have method make that doesn't force (takes no params), and new >method make_force that forces build

In fact, I think that idea of this parameter is great. I had no such option in my home made analog of depgraph.

You write rule, and while you develop method that rule execute you leave it as force=true, then you need just change force=false and developing done. After that if need some modification in rule, you again force=true while debugging.

Have one method for force build and another for not force require some reading of documentation, if just one method all things are more simple, in my humble opinion.

derekdreery commented 7 years ago

If you do a PR for these changes I'll happily make a new version on crates.io :)