build: remove pre-push hooks

[EdJoPaTo]

I assume the ci task is also run by GitHub actions so this PR should ensure the checks on GitHub should still test these things.

Personally I stopped using the git hook / cargo make ci and use the standard commands cargo +nightly fmt, cargo clippy‘ (--all-features) andcargo testwith (--liband/or--all-features` as they are similar across all projects. GitHub action checks will run the full tests anyway and find mistakes I made so having local stuff quicker is what I prefer. This PR goes into a good direction for that.

(Also, running cargo test excluding the doctests is faster on my devices compared to nextest.)

I would also reorder the Makefile to keep the simple things first and then the more advanced ones. At the top would be a cargo make quick running fmt, clippy und test --lib as a minimal test command. Then there might be fmt, clippy —-all-features and test --all-features as more complete check run. Then more non standard ways can follow. Using typos for example. That way the makefile is more easy to follow what will be run. The contribution guide should suggest cargo make quick and all-features and suggest a look into the Makefile for more in depth commands. I also liked that it stated make ci being also the command being run on CI. So it’s easy to run the full CI locally.

ratatui requiring me to use typos, make and nextest (especially automatically installing them without any confirmation on my side while using git push) was one of the annoying things at the start of contributing to ratatui. These tools are neat and should be available in an easy way locally too, but it should be easy to use the cargo standard tooling while the full tests are running online in actions anyway.

Grouping the makefile better in the suggested ways should help to easily see what will happen when running it. Then it’s easy to see what will be done by executing the command.

There should not be a push hook especially one that installs random stuff without confirmation. When a person doesn’t run full checks locally the GitHub running checks will still catch these things and show it to the users. Then I can choose to install and run typos/nextest/whatever locally because it failed a check online.

[joshka]

I assume the ci task is also run by GitHub actions so this PR should ensure the checks on GitHub should still test these things.

It's not. This target could be called pre-push

The GitHub CI process runs the individual makefile targets rather than any higher level one, so that these show up as parts of the GitHub Actions, so that they can be parallelized, and so that they can run against multiple operating systems.

Personally I stopped using the git hook / cargo make ci and use the standard commands cargo +nightly fmt, cargo clippy‘ (--all-features) and cargo test with (--liband/or--all-features` as they are similar across all projects.

The windows vs non-windows story is a pain for --all-features.

(Also, running cargo test excluding the doctests is faster on my devices compared to nextest.)

0.84s vs 4.49s on my machine. Got some contrasting numbers?

I would also reorder the Makefile to keep the simple things first and then the more advanced ones. At the top would be a cargo make quick running fmt, clippy und test --lib as a minimal test command. Then there might be fmt, clippy —-all-features and test --all-features as more complete check run.

Are these visibly different without the --all-features?

especially automatically installing them without any confirmation on my side while using git push)

Yeah, this is probably bad

Grouping the makefile better in the suggested ways should help to easily see what will happen when running it. Then it’s easy to see what will be done by executing the command.

I have no real concern over the order of the makefile. If there's a better order go for it. Basically the makefile works fine, doesn't greatly cause problems, so there's no real value in "fixing it". If there was, I'd be inclined to replace the makefile with a cargo xtask package, and dispense with needing cargo-make at all, but it's just not a high priority.

I wonder if there's an easy way to git push, and then watch the github checks succeed / fail in the terminal. Somone 'aughta make an extension for gh that does that.

[a-kenji]

gh does support watching, I find it convenient:

gh pr checks --fail-fast --watch

gh pr checks --help

[joshka]

gh does support watching, I find it convenient:

Awesome. Thanks for pointing that out, you wouldn't believe how difficult it is to find the right google terms to get to this "github actions tui" etc.

[joshka]

Changed this PR to just remove the hooks. I usually skip running them locally and Ed just disables them entirely.

[a-kenji]

Thank you, I also always disabled them :+1: .

[EdJoPaTo]

(Also, running cargo test excluding the doctests is faster on my devices compared to nextest.)

0.84s vs 4.49s on my machine. Got some contrasting numbers?

$ hyperfine --shell=none --warmup 1 --prepare 'touch src/*.rs' 'cargo test --lib --tests --all-features' 'cargo nextest run --all-features'
Benchmark 1: cargo test --lib --tests --all-features
  Time (mean ± σ):      1.405 s ±  0.031 s    [User: 3.149 s, System: 0.206 s]
  Range (min … max):    1.381 s …  1.487 s    10 runs

Benchmark 2: cargo nextest run --all-features
  Time (mean ± σ):      5.540 s ±  0.057 s    [User: 9.537 s, System: 9.129 s]
  Range (min … max):    5.457 s …  5.618 s    10 runs

Summary
  cargo test --lib --tests --all-features ran
    3.94 ± 0.10 times faster than cargo nextest run --all-features

[joshka]

5s is definitely in the realm of fast enough for a manual run IMHO, but < 1s is fast enough for a workflow that runs tests on every save. Both seem reasonable use cases.