Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: move cargo fmt to pre-commit script #2163

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ task:
- FreeBSD 14 amd64 & i686
- Linux x86_64
- macOS aarch64
- Rust Formatter
- OpenBSD x86_64
- Minver
- Rust Stable
Expand Down Expand Up @@ -152,7 +151,6 @@ task:
- FreeBSD 14 amd64 & i686
- Linux x86_64
- macOS aarch64
- Rust Formatter
- OpenBSD x86_64
- Minver
- Rust Stable
Expand All @@ -172,7 +170,6 @@ task:
- FreeBSD 14 amd64 & i686
- Linux x86_64
- macOS aarch64
- Rust Formatter
- OpenBSD x86_64
- Minver
- Rust Stable
Expand Down Expand Up @@ -205,7 +202,6 @@ task:
- FreeBSD 14 amd64 & i686
- Linux x86_64
- macOS aarch64
- Rust Formatter
- OpenBSD x86_64
- Minver
- Rust Stable
Expand Down Expand Up @@ -275,7 +271,6 @@ task:
- FreeBSD 14 amd64 & i686
- Linux x86_64
- macOS aarch64
- Rust Formatter
- OpenBSD x86_64
- Minver
- Rust Stable
Expand Down Expand Up @@ -308,7 +303,6 @@ task:
- FreeBSD 14 amd64 & i686
- Linux x86_64
- macOS aarch64
- Rust Formatter
- OpenBSD x86_64
- Minver
- Rust Stable
Expand All @@ -322,7 +316,6 @@ task:
- FreeBSD 14 amd64 & i686
- Linux x86_64
- macOS aarch64
- Rust Formatter
- OpenBSD x86_64
- Minver
- Rust Stable
Expand All @@ -333,7 +326,6 @@ task:
- FreeBSD 14 amd64 & i686
- Linux x86_64
- macOS aarch64
- Rust Formatter
- OpenBSD x86_64
- Minver
- Rust Stable
Expand All @@ -359,12 +351,3 @@ task:
check_script:
- cargo check
before_cache_script: rm -rf $CARGO_HOME/registry/index

# Tasks that checks if the code is formatted right using `cargo fmt` tool
task:
name: Rust Formatter
container:
image: rust:latest
cpu: 1
setup_script: rustup component add rustfmt
test_script: cargo fmt --all -- --check **/*.rs
55 changes: 54 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,50 @@ pull' model described there.

Please make pull requests against the `master` branch.

## Check your code format

We use [git pre-commit hook](https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks)
to automate code formatting. However, you need to manually link our script to the correct
location first, assume you are in the root of the nix project:

```sh
ln -s ../../pre-commit.sh .git/hooks/pre-commit
```

Let's add some a commit:

```shell
$ git add {CHANGES}
$ git commit -m "Some commit message"
```

If your code is not formatted, then you will see a message like this:

```shell
INFO: Checking code format...
WARN: Unformatted code detected
INFO: Formatting...
FAIL: git commit ABORTED, please have a look and run git add/commit again
```

Our `pre-commit` script will find code that is not formatted and format it for
you, your commit operation will be aborted, add the changes and commit again:

```shell
$ git add {CHANGES_MADE_BY_THE_FORMATTER}
$ git commit -m "Some commit message"
INFO: Checking code format...
INFO: Check passed
```
Since the code has been formatted by the formatter, the check will pass and you
are good to go!

## CHANGELOG

If you change the API by way of adding, removing or changing something or if
you fix a bug, please add an appropriate note, every note should be a new markdown
file under the [changelog directory][cl] stating the change made by your pull request,
the filename should be in the following foramt:
the filename should be in the following format:

```
<PULL_REQUEST_ID>.<TYPE>.md
Expand Down Expand Up @@ -109,6 +149,19 @@ possible! Other tests cannot be run under QEMU, which is used for some
architectures. To skip them, add a `#[cfg_attr(qemu, ignore)]` attribute to
the test.

### Skip the CI

It is highly recommended to skip the CI if your PR doesn't change nix's code
as this will save our CI usage.

To do this, add:

```
[skip ci]
```
to your PR description, just like [#2158](https://github.com/nix-rust/nix/pull/2158)
does.

## GitHub Merge Queues

We use GitHub merge queues to ensure that subtle merge conflicts won't result
Expand Down
36 changes: 36 additions & 0 deletions pre-commit.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#!/bin/sh

function red() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function is bash syntax, not sh.

Suggested change
function red() {
red() {

echo "\033[0;31m$@\033[0m"
}

function green() {
echo "\033[0;32m$@\033[0m"
}

function byellow() {
echo "\033[1;33m$@\033[0m"
}

CHANGED_BY_CARGO_FMT=false


echo -e "$(green INFO): Checking code format..."

cargo fmt --all -q -- --check 2>/dev/null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This command doesn't actually work. I just tested it by screwing up the format in src/sys/aio.rs, and this command didn't complain. I think it's caused by this bug: rust-lang/rustfmt#3253 . That's why we need to do cargo fmt --all -- --check **/*.rs

if [ $? -ne 0 ]; then
echo -e "$(byellow WARN): Unformatted code detected"
echo -e "$(green INFO): Formatting..."
cargo fmt --all
CHANGED_BY_CARGO_FMT=true
fi

if ${CHANGED_BY_CARGO_FMT};
then
echo -e "$(red FAIL): git commit $(red ABORTED), please have a look and run git add/commit again"
exit 1
else
echo -e "$(green INFO): Check passed"
fi

exit 0