karolsluszniak / ex_check

One task to efficiently run all code analysis & testing tools in an Elixir project. Born out of 💜 to Elixir and pragmatism.
https://hex.pm/packages/ex_check
MIT License
308 stars 11 forks source link

Add prechecks #10

Closed alanvardy closed 4 years ago

alanvardy commented 4 years ago

Hi Karol!

I've been really enjoying using this library and consider it a standard part of my tooling.

I am wondering if I can contribute to your library? My idea is to add additional "prechecks" that are run synchronously after run_compiler/1 and before run_others/2 https://github.com/karolsluszniak/ex_check/blob/db4f20ec5461be99a8e88eb6bea27e36a0e6eaeb/lib/ex_check/check.ex#L14

My use case is that I would like to run:

My reasoning is that a credo check takes only a second or two, so if it fails I would rather address it immediately rather than wait a couple of minutes for the whole test suite to run.

In order to not break your current API, I was thinking about another list of :prechecks that uses the same format as :tools in .check.exs.

[
  ## all available options with default values (see `mix check` docs for description)
  # skipped: true,
  # exit_status: true,
  # parallel: true,

  # Tools to run synchronously before running the tools below
  prechecks: [
    # Have same options as the tools below
    # A non-zero exit code will halt all further execution
  ]

  ## list of tools (see `mix check` docs for defaults)
  tools: [
    ## curated tools may be disabled (e.g. the check for compilation warnings)
    # {:compiler, false},

    ## ...or adjusted (e.g. use one-line formatter for more compact credo output)
    # {:credo, command: "mix credo --format oneline"},

    ## custom new tools may be added (mix tasks or arbitrary commands)
    # {:my_mix_check, command: "mix release", env: %{"MIX_ENV" => "prod"}},
    # {:my_arbitrary_check, command: "npm test", cd: "assets"},

    # {:my_arbitrary_script, command: ["my_script", "argument with spaces"], cd: "scripts"}
  ]
]

I am not sure about the naming, it can probably be improved. Is this a desirable addition to your library?

karolsluszniak commented 4 years ago

Hey, glad ex_check is useful for you!

Yeah, I've had this case in mind for some time and in fact an unmerged #5 that addresses this concern was hanging for quite some time, waiting for some final touches. I've reviewed & tested it today and I've decided to merge it. There was one complex concern with umbrella apps that I wanted to tackle still, but decided to leave it for another day.

My idea was to offer a generic dependency solution instead of one that handles just a specific cases - like it would be if we'd go with your proposed solution of prechecks. This enables workflows in which you decide to make only some tools depend on others to succeed or to create a chain in which C+D execute only if B executes and B executes only if A executes (A → B → (C + D).

Anyway, it's on master and I'll release it as 0.12 soon. With it, you'll have to do following adjustments in .check.exs in order to achieve your "credo before all" case.

[
  tools: [
    {:ex_unit, deps: [{:credo, status: :ok}]},
    {:dialyzer, deps: [{:credo, status: :ok}]},
    # ...and so forth for all the tools that you want to run only after credo is OK
  ]
]

In similar way you may keep short ones in the "early game" e.g. formatter and only run ExUnit and Dialyzer after the fast ones are fine. Of course parallelism still applies both to early and late tools.

Let me know how it works for you!

alanvardy commented 4 years ago

Fantastic, thank you!

alanvardy commented 4 years ago

Just tested it, worked perfectly. This massively improves my tooling experience across all my projects. Cheers!