diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 7641aed735..9ba7213405 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -19,6 +19,13 @@ Make sure you've run and fixed any issues with these commands: - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass +> **Note** +> from `nushell` you can also use the `toolkit` as follows +> ```bash +> use toolkit.nu # or use an `env_change` hook to activate it automatically +> toolkit check pr +> ``` + # After Submitting If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. diff --git a/toolkit.nu b/toolkit.nu new file mode 100644 index 0000000000..6247a8369f --- /dev/null +++ b/toolkit.nu @@ -0,0 +1,141 @@ +# this module regroups a bunch of development tools to make the development +# process easier for anyone. +# +# the main purpose of `toolkit` is to offer an easy to use interface for the +# developer during a PR cycle, namely to (**1**) format the source base, +# (**2**) catch classical flaws in the new changes with *clippy* and (**3**) +# make sure all the tests pass. + +# check standard code formatting and apply the changes +export def fmt [ + --check: bool # do not apply the format changes, only check the syntax +] { + if ($check) { + cargo fmt --all -- --check + } else { + cargo fmt --all + } +} + +# check that you're using the standard code style +# +# > it is important to make `clippy` happy :relieved: +export def clippy [] { + cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect +} + +# check that all the tests pass +export def test [ + --fast: bool # use the "nextext" `cargo` subcommand to speed up the tests (see [`cargo-nextest`](https://nexte.st/) and [`nextest-rs/nextest`](https://github.com/nextest-rs/nextest)) +] { + if ($fast) { + cargo nextest --workspace + } else { + cargo test --workspace + } +} + +# run all the necessary checks and tests to submit a perfect PR +# +# # Example +# let us say we apply a change that +# - breaks the formatting, e.g. with extra newlines everywhere +# - makes clippy sad, e.g. by adding unnecessary string conversions with `.to_string()` +# - breaks the tests by output bad string data from a data structure conversion +# +# > the following diff breaks all of the three checks! +# > ```diff +# > diff --git a/crates/nu-command/src/formats/to/nuon.rs b/crates/nu-command/src/formats/to/nuon.rs +# > index abe34c054..927d6a3de 100644 +# > --- a/crates/nu-command/src/formats/to/nuon.rs +# > +++ b/crates/nu-command/src/formats/to/nuon.rs +# > @@ -131,7 +131,8 @@ pub fn value_to_string(v: &Value, span: Span) -> Result { +# > } +# > }) +# > .collect(); +# > - let headers_output = headers.join(", "); +# > + let headers_output = headers.join(&format!("x {}", "") +# > + .to_string()); +# > +# > let mut table_output = vec![]; +# > for val in vals { +# > ``` +# +# - we run the toolkit once and it fails... +# ```nushell +# >_ toolkit check pr +# running `toolkit fmt` +# Diff in /home/amtoine/.local/share/git/store/github.com/amtoine/nushell/crates/nu-command/src/formats/to/nuon.rs at line 131: +# } +# }) +# .collect(); +# - let headers_output = headers.join(&format!("x {}", "") +# - .to_string()); +# + let headers_output = headers.join(&format!("x {}", "").to_string()); +# +# let mut table_output = vec![]; +# for val in vals { +# +# please run toolkit fmt to fix the formatting +# ``` +# - we run `toolkit fmt` as proposed and rerun the toolkit... to see clippy is sad... +# ```nushell +# running `toolkit fmt` +# running `toolkit clippy` +# ... +# error: redundant clone +# --> crates/nu-command/src/formats/to/nuon.rs:134:71 +# | +# 134 | let headers_output = headers.join(&format!("x {}", "").to_string()); +# | ^^^^^^^^^^^^ help: remove this +# | +# note: this value is dropped without further use +# --> crates/nu-command/src/formats/to/nuon.rs:134:52 +# | +# 134 | let headers_output = headers.join(&format!("x {}", "").to_string()); +# | ^^^^^^^^^^^^^^^^^^^ +# = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone +# = note: `-D clippy::redundant-clone` implied by `-D warnings` +# +# error: could not compile `nu-command` due to previous error +# ``` +# - we remove the useless `.to_string()`, and in that cases, the whole format is useless, only `"x "` is usefull! +# but now the tests do not pass :sob: +# ```nushell +# running `toolkit fmt` +# running `toolkit clippy` +# ... +# running `toolkit test` +# ... +# failures: +# commands::insert::insert_uses_enumerate_index +# commands::merge::multi_row_table_overwrite +# commands::merge::single_row_table_no_overwrite +# commands::merge::single_row_table_overwrite +# commands::update::update_uses_enumerate_index +# commands::upsert::upsert_uses_enumerate_index_inserting +# commands::upsert::upsert_uses_enumerate_index_updating +# commands::where_::where_uses_enumerate_index +# format_conversions::nuon::does_not_quote_strings_unnecessarily +# format_conversions::nuon::to_nuon_table +# ``` +# - finally let's fix the tests by removing the `x`, essentially removing the whole diff we applied at the top! +# +# now the whole `toolkit check pr` passes! :tada: +export def "check pr" [ + --fast: bool # use the "nextext" `cargo` subcommand to speed up the tests (see [`cargo-nextest`](https://nexte.st/) and [`nextest-rs/nextest`](https://github.com/nextest-rs/nextest)) +] { + print "running `toolkit fmt`" + try { + fmt --check + } catch { + print $"\nplease run (ansi default_dimmed)(ansi default_italic)toolkit fmt(ansi reset) to fix the formatting" + return + } + + print "running `toolkit clippy`" + clippy + + print "running `toolkit test`" + if $fast { test --fast } else { test } +}