forked from extern/nushell
FEATURE: wrap the dev instructions from the PR template for easier use (#8152)
i was writing #8148 and came to the "_Test + Formatting_" section of the PR template. i felt the developer instructions could be wrapped up in a common easy to use format, e.g. a `Makefile`, to be used with a few-words command only, e.g. `make fmt` or `make clippy`, instead of the long commands in the PR template 🤔 > **Note** > in case you guys do not want to add a `Makefile` to the `nushell` source, that PR can be discarded 😌 # Description this PR - adds the few rules from the PR template to a new `Makefile` - replaces the instructions in the PR template from the full commands to the new `make` rules # User-Facing Changes - _none for the regular user_ - i believe easier to test PRs for the developer, allowing one not to realy on knowing the long commands or using the shell history to run them again 😋 # Tests + Formatting _nothing to test_ # After Submitting maybe mention that in `CONTRIBUTING.md`?
This commit is contained in:
Antoine Stevan
GitHub
parent
2bef85a913
commit
7e82f8d9b5
7
.github/pull_request_template.md
vendored
7
.github/pull_request_template.md
vendored
@ -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.
|
||||
|
||||
141
toolkit.nu
Normal file
141
toolkit.nu
Normal file
@ -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<String, ShellError> {
|
||||
# > }
|
||||
# > })
|
||||
# > .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 }
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user